No link-hints for click handlers, registered with jQuery.live

maximbaz commented 9 years ago

Consider the following example:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>Link-hints for jquery.live</title>
    <style>.spoiler_title { text-decoration: underline; cursor: pointer; }</style>
  </head>
  <body>
    <div class="html_format">
      <div class="spoiler">
        <b class="spoiler_title">Spoilers!</b>
        <div class="spoiler_text" style="display: none;">
          <span>Hidden text is here!</span>
        </div>
      </div>
    </div>

    <script src="https://code.jquery.com/jquery-1.8.3.min.js"></script>
    <script>
      $(".html_format .spoiler_title").live("click", function() {
        var spoiler = $(this).closest(".spoiler");
        $("> .spoiler_text", spoiler).slideToggle();
        spoiler.toggleClass("spoiler_open");
      });
    </script>
  </body>
</html>

Press f to show link-hints. Expected: There is a link-hint to expand/collapse spoiler. Actual: There is none.

Tested on both master and post-1.46.

The following code was extracted from the popular Russian-speaking collaborative blog habrahabr.ru, so the impacted number of people for this particular example is rather high.

mrmr1993 commented 9 years ago

JSFiddle here. Looking at the .live() documentation, it looks like the function is depreciated, so hopefully it will become less of an issue as time goes on.

We can get the registered event listeners from jQuery._data(document, "events") (an internal jQuery API), run in the page (not our content script) context. It would probably be better to hook the jQuery listener registration functions, in a similar style to we do with addEventListener in #1167.

Edit: To communicate the information to the content script, we'll probably want to use a custom event, since (at least in this case) we're dealing with a selector rather than any specific element.

maximbaz commented 9 years ago

It is deprecated in favor of .on(), which also needs to be patched (see JSFiddle v2).

I like the idea to hook jQuery functions like #1167.

mrmr1993 commented 9 years ago

It is deprecated in favor of .on(), which also needs to be patched (see JSFiddle v2).

That version works fine for me using #1167. You need to gf to the frame.

Edit: inserted italic text

maximbaz commented 9 years ago

OK, so let this particular issue be only about .live() then.

I can see it was deprecated since jQuery 1.7 and removed in 1.9, but as it still being used in such popular sites, we should support it.

mrmr1993 commented 9 years ago

... as it still being used in such popular sites, we should support it.

@z0rch can you have a go at implementing it?

maximbaz commented 9 years ago

I'll have a look in the next year :smile: Thanks for #1405, by the way.

maximbaz commented 9 years ago

Just occurred to me, that in general case

$(selector).live("click", handlerFn);

is translated into

$(document).on("click", selector, handlerFn);

rather than

$(selector).on("click", handlerFn);

I feel that's where is the difference, handler is assigned to document rather than to a particular element. I updated the JSFiddle (see v3) to reflect this thought.

Will have a more detailed look later on.

mrmr1993 commented 9 years ago

Ahh, nice catch. We should probably guard against

$(element).on("click", selector, handlerFn);

too then, by hooking the on method rather than querying event listeners on document.

Looking forward to seeing your work on this!

maximbaz commented 9 years ago

Hey @mrmr1993, I'd like to hear your suggestions on the approach to choose.

First of all, keep in mind that the syntax $(document).on("click", selector, handlerFn) is used to setup a handler for elements, that do not necessarily exist at the time of setting up the handler.

This means that we need some way to remember the selector, and find matching elements by ourselves when a user hits f, just like jQuery itself would do.

Now, in order to hook the .on() method, we need to place the hook after jQuery is loaded, but before the first use of .on() method. In general case, these two things may exist in one js file (minimized and concatenated), so I'm not even sure how to intercept our hook in-between.

To give you a better insight, here is what roughly happens inside jQuery.on():

Link between selector and handlerFn is saved in private variable, let's name it privateHandlers
A general event listener for click is assigned to document, with no information about selector or handlerFn
When a click event occurs anywhere on the page and bubbles up to document, jQuery uses privateHandlers to call handlerFn only if the chain of clicked events contains an element, matching the selector.

As you can see, even if we switch for a moment to think about overloading document::addEventListener (just like you've been hooking Element::addEventListener), we cannot extract the information about selector from there.

Using this information, do you have any suggestions on how to retrieve those selectors? What kind of hook would be appropriate for us to set, so that we can capture them? Maybe you can see an another idea, that I'm missing here?

mrmr1993 commented 9 years ago

Now, in order to hook the .on() method, we need to place the hook after jQuery is loaded, but before the first use of .on() method.

I've updated the JSFiddle with my suggested way of hooking the jQuery function. It uses Object.defineProperty on window to overwrite the jQuery implementation of on as it's set, so we don't have to worry about the timing.

maximbaz commented 9 years ago

Properties... beautiful! Thanks for the hint :+1:

philc / vimium

No link-hints for click handlers, registered with jQuery.live #1404