Loading ...
Sorry, an error occurred while loading the content.

Event.fireLegacyEvent() uses Function.call()?

Expand Messages
  • Peter Michaux
    Hi, I ve still been reading the Yahoo! UI event library in detail. I found a bit of suspicious code. Here are my thoughts but I d think anyone should double
    Message 1 of 5 , Sep 3, 2006
    • 0 Attachment
      Hi,

      I've still been reading the Yahoo! UI event library in detail. I found
      a bit of suspicious code. Here are my thoughts but I'd think anyone
      should double check the details :)

      ----------------------------

      There is a lot of effort in this libray to support browsers that do
      not support the modern event models that use
      Element.addEventListener() or Element.attachEvent(). These old
      browsers are before NS6 or IE4. These browsers will have
      useLegacyEvent() return true because of this snip

      useLegacyEvent: function(el, sType) {

      if (!el.addEventListener && !el.attachEvent) {
      return true;

      But these very same browsers did not have Function.call() so the
      following will not work.

      fireLegacyEvent: function(e, legacyIndex) {
      // [sinp]
      var ret = li[this.WFN].call(scope, e);

      I believe if this was changed to the following you could get support
      back to NS4 and IE 5.5

      var ret = li[this.WFN].apply(scope, [e]);

      Browsers back to IE 4 will pass the el.attachEvent test and not use
      legacy events. The IE browsers that would use legaciy events do not
      have support for apply(). So any IE browser that passes the "if
      (!el.addEventListener && !el.attachEvent) {" test will not be able to
      use the apply() anyway in fireLegacyEvent. So shortening the test to
      "if (!el.addEventListener) {" would be sufficient.

      ----------------------------

      I'm sure I've written that logic in a confusing manner but the change
      to apply() probably is a good idea. The removal of "&&!el.attachEvent"
      is optional as it doesn't seem like it would make the script work in
      any more browsers than it already does.

      The best this library can do is full support back to NS6 and IE 5.5.
      Back to NS4 will sort of work but things Event.preventDefault() won't
      work and developers may not realize that (however who cares about
      NS4). Any support for browsers before IE 5.5 is not possible because
      of the use of call() or apply().

      Peter
    • Bart King
      Hello, While this is a good discussion about support for
      Message 2 of 5 , Sep 4, 2006
      • 0 Attachment
        Hello,

        While this is a good discussion about support for < IE5.5 and < NS8.1,
        the Yahoo guys have already indirectly said that they are unlikely to be
        supporting old browsers such as these as outlined at
        http://developer.yahoo.com/yui/articles/gbs/gbs_browser-chart.html.

        Also, I don't think anyone really cares these days. Having to support
        antique technology such as IE4 and NS4 just means a reasonable amount of
        work for nothing. A quick glance at my stats on my web site over the
        last three days, out of 379846 hits on for MSIE, less than 1000 are for
        MSIE 5.5 or less (NS4 has 317 and MSIE 4.0 has exactly four :).

        Cheers,
        --
        Bart King
        http://www.bart666.com -- +44 781 219 5654


        -----Original Message-----
        From: ydn-javascript@yahoogroups.com
        [mailto:ydn-javascript@yahoogroups.com] On Behalf Of Peter Michaux
        Sent: 04 September 2006 00:10
        To: ydn-javascript@yahoogroups.com
        Subject: [ydn-javascript] Event.fireLegacyEvent() uses Function.call()?

        <snipped>
      • Peter Michaux
        ... Hi Bart, I agree completely with you. Those browsers are old now. It is just the inconsistency of this conditional to trigger legacy events if
        Message 3 of 5 , Sep 4, 2006
        • 0 Attachment
          On 9/4/06, Bart King <bart@...> wrote:
          >
          > Hello,
          >
          > While this is a good discussion about support for < IE5.5 and < NS8.1,
          > the Yahoo guys have already indirectly said that they are unlikely to be
          > supporting old browsers such as these as outlined at
          > http://developer.yahoo.com/yui/articles/gbs/gbs_browser-chart.html.
          >
          > Also, I don't think anyone really cares these days. Having to support
          > antique technology such as IE4 and NS4 just means a reasonable amount of
          > work for nothing. A quick glance at my stats on my web site over the
          > last three days, out of 379846 hits on for MSIE, less than 1000 are for
          > MSIE 5.5 or less (NS4 has 317 and MSIE 4.0 has exactly four :).

          Hi Bart,

          I agree completely with you. Those browsers are old now. It is just
          the inconsistency of this conditional to trigger legacy events

          if (!el.addEventListener && !el.attachEvent)

          with the fact that these very same browsers don't have call() anyway.
          If the three uses of .call() were changed to .apply() then there would
          be a little bit of consistency.

          I think it would be best to leave call() how it is in the file and
          remove the conditional to trigger the legacy events in these old
          browsers. This is because these old browsers won't be supported by
          other parts of the library anyway (eg preventDefault() etc).

          Peter
        • Matt Warden
          ... Admittedly I do not fully understand what you re saying here, but I did want to point out that I don t think the check for addEventListener and attachEvent
          Message 4 of 5 , Sep 4, 2006
          • 0 Attachment
            On 9/4/06, Peter Michaux <petermichaux@...> wrote:
            > if (!el.addEventListener && !el.attachEvent)
            >
            > with the fact that these very same browsers don't have call() anyway.
            > If the three uses of .call() were changed to .apply() then there would
            > be a little bit of consistency.

            Admittedly I do not fully understand what you're saying here, but I
            did want to point out that I don't think the check for
            addEventListener and attachEvent is a browser sniff. Making
            assumptions like "if A is not supported, this is Browser X, and
            therefore B is also not supported" is something developers have
            frowned upon for a good while now, favoring a strict method/function
            detection approach (e.g. "if A is not supported, then we don't use A.
            Separately, we check if B is supported before using B").

            In light of this, would you instead suggest that we perform a
            detection on Function.call, or are you simply saying that
            Function.apply provides the same functionality (as used here) with
            broader support?

            --
            Matt Warden
            Cleveland, OH, USA
            http://mattwarden.com


            This email proudly and graciously contributes to entropy.
          • Peter Michaux
            Hi Matt, Thanks for the reply. ... Good point. Well said. This logic is unfortunately used in YUI event.js. If the browser is detected to be Safari then click
            Message 5 of 5 , Sep 4, 2006
            • 0 Attachment
              Hi Matt,

              Thanks for the reply.

              On 9/4/06, Matt Warden <mwarden@...> wrote:
              >
              > On 9/4/06, Peter Michaux <petermichaux@...> wrote:
              > > if (!el.addEventListener && !el.attachEvent)
              > >
              > > with the fact that these very same browsers don't have call() anyway.
              > > If the three uses of .call() were changed to .apply() then there would
              > > be a little bit of consistency.
              >
              > Admittedly I do not fully understand what you're saying here, but I
              > did want to point out that I don't think the check for
              > addEventListener and attachEvent is a browser sniff. Making
              > assumptions like "if A is not supported, this is Browser X, and
              > therefore B is also not supported" is something developers have
              > frowned upon for a good while now, favoring a strict method/function
              > detection approach (e.g. "if A is not supported, then we don't use A.
              > Separately, we check if B is supported before using B").

              Good point. Well said. This logic is unfortunately used in YUI
              event.js. If the browser is detected to be Safari then click and
              dblclick events will use legacy events. However, I tested and it seems
              that Safari 2 handles these events just fine with DOM2 events. So if
              the library's use of legacy events handle click and dblclick without
              any compromise in Safari 1.3 and 2.0, then why not just use legacy
              events for click and dblclick in all browsers and eliminate the
              browser sniff? If there is compromise what is it and can it be added
              to the documentation?


              > In light of this, would you instead suggest that we perform a
              > detection on Function.call, or are you simply saying that
              > Function.apply provides the same functionality (as used here) with
              > broader support?

              I am saying Function.apply() provides the same functionality with
              broader support.

              All the rambling was just trying to show that the event library seems
              to be trying to support the older browsers without Function.call() to
              a certain degree but that using Function.call() was the limiting
              factor. Switching to Function.apply() lets other parts of the library
              be the limiting factors for even older browsers.

              Staying with Function.call() isn't a bad idea either. Since
              Function.call() is essential to the library there is no need to define
              YAHOO.util.Event if Function.call() doesn't exist.

              I suppose not much of this really matters in the reai world. I was
              simply analyzing the library to see how far back the library would
              support.

              I personally am going to stay with Function.call() in my trimmed
              version of YUI! event.js and let the browser error if the browser
              doesn't have it. I'm comfortable with this since Function.call() is a
              core JavaScript function and not specifically a client-side function.
              I have also removed all of the legacy event stuff and changed the
              getPageX methods. To accomodate Safari <=1.3, I simply won't use
              dblclick events or try to preventDefault for single clicks on links in
              DOM2 event handlers. With these changes and limits I have removed all
              browser sniffing (ie navigator.userAgent). Also this cuts the library
              from >8KB to ~3.5KB. Hopefully less code equals less maintaince and
              none of these changes limit my needs.

              Peter
            Your message has been successfully submitted and would be delivered to recipients shortly.