Closed Bug 1355451 Opened 7 years ago Closed 7 years ago

Speculatively connect to websites on mousedown on an awesomebar item

Categories

(Firefox :: Address Bar, enhancement, P1)

Product:
Firefox
Component:
Address Bar
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: florian, Assigned: jj.evelyn)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-performance][fxsearch])

Attachments

(1 file)

Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.
59 bytes, text/x-review-board-request
mak
: review+
Details 
Pressing the mouse on a suggested result in the awesomebar (either a suggestion or something from the history) is a strong signal that the page is about to be loaded. We should prepare the network connection without waiting for the click event.
Hi Florian, should this bug be part of the Performance team backlog?
Flags: needinfo?(florian)
(In reply to Marco Mucci [:MarcoM] from comment #1)
> Hi Florian, should this bug be part of the Performance team backlog?

For now, let's say yes. The search team may decide to take it though.
Flags: needinfo?(florian)
Whiteboard: [photon][fxsearch] → [photon-performance][fxsearch]
Blocks: photon-performance-triage
Flags: qe-verify?
Priority: -- → P2
Blocks: photon-perf-spec-connect
No longer blocks: photon-performance-triage
Priority: P2 → P3
Whiteboard: [photon-performance][fxsearch] → [reserve-photon-performance][fxsearch]
Flags: qe-verify? → qe-verify-
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

Hi :mak, in this patch I tried to NOT touch nsAutoCompleteController code so we handle the speculative connect setup in the JS. I see the mouseup event is handled by |nsAutoCompleteController::HandleEnter|, but my case is simpler and doesn't need to hack nsAutoCompleteController I guess. Let me know if I missed something. Thanks!
Attachment #8885660 - Flags: feedback?(mak77)
Assignee: nobody → ehung
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review162078

::: browser/base/content/urlbarBindings.xml:1689
(Diff revision 1)
> +          if (aEvent.button == 2) {
> +            // Ignore right-clicks.
> +            return;
> +          }
> +          let controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);
> +          let uri = controller.getFinalCompleteValueAt(this.selectedIndex);

What about moz-action uris (anything that is not history/bookmarks)? we should really not try to speculative connect to them.
It may also be a search engine for which we must still build an url.
it may also be a one-off button.

I only see 2 ways to handle this:

1. the code in handleCommand is the only code that properly extracts the final URL
http://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/browser/base/content/urlbarBindings.xml#521
We'd have to invoke this code earlier, and later in handleCommand if we already did the "calculation" skip the whole thing.
2. limit this to some specific cases for which we can easily extract the url.

::: toolkit/content/widgets/autocomplete.xml:2611
(Diff revision 1)
>  
>          this.mLastMoveTime = Date.now();
>        ]]>
>        </handler>
> +
> +      <handler event="mousedown">

If I read it correctly, urlbar-rich-result-popup already has a mousedown handler, couldn't we add to that?
from there you should also have access to the urlbar binding through this.input.

The reason is that I'm truing to be a little bit more stricter about changes to the toolkit autocomplete that are only required by a single consumer, we were too permissive in the past, and it's becoming unmanageable :(
Attachment #8885660 - Flags: review-
Attachment #8885660 - Flags: feedback?(mak77)
(In reply to Marco Bonardo [::mak] from comment #5)
> Comment on attachment 8885660 [details]
> Bug 1355451 - Speculative connect to websites on mousedown on awesomebar
> item.
> 
> https://reviewboard.mozilla.org/r/156512/#review162078
> 
> ::: browser/base/content/urlbarBindings.xml:1689
> (Diff revision 1)
> > +          if (aEvent.button == 2) {
> > +            // Ignore right-clicks.
> > +            return;
> > +          }
> > +          let controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);
> > +          let uri = controller.getFinalCompleteValueAt(this.selectedIndex);
> 
> What about moz-action uris (anything that is not history/bookmarks)? we
> should really not try to speculative connect to them.
> It may also be a search engine for which we must still build an url.
> it may also be a one-off button.
> 
> I only see 2 ways to handle this:
> 
> 1. the code in handleCommand is the only code that properly extracts the
> final URL
> http://searchfox.org/mozilla-central/rev/
> cef8389c687203085dc6b52de2fbd0260d7495bf/browser/base/content/urlbarBindings.
> xml#521
> We'd have to invoke this code earlier, and later in handleCommand if we
> already did the "calculation" skip the whole thing.
> 2. limit this to some specific cases for which we can easily extract the url.
> 

Update the conversation on IRC:

It seems that some moz-action cases don't need to speculative connect. 
Here is the list:
- visiturl: no, because the url is unknown, e.g. about:config or http://mozilla/run
- remotetab: yes, a real url which opened on a remote device
- keyword: yes, the keyword maps to a real site, e.g. mdn
- switchtab: no, the site is already opened
- searchengine: no? we may already have a connection opened by bug 1355443
- extension: not sure. (I didn't figure out a real example)

I might be able to get the actual url from remotetab and keyword easily. Otherwise, we could simply limit to direct http(s) case which still have decent coverage, and improve moz-action part later.
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review162078

> What about moz-action uris (anything that is not history/bookmarks)? we should really not try to speculative connect to them.
> It may also be a search engine for which we must still build an url.
> it may also be a one-off button.
> 
> I only see 2 ways to handle this:
> 
> 1. the code in handleCommand is the only code that properly extracts the final URL
> http://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/browser/base/content/urlbarBindings.xml#521
> We'd have to invoke this code earlier, and later in handleCommand if we already did the "calculation" skip the whole thing.
> 2. limit this to some specific cases for which we can easily extract the url.

I limit the cases to only for http(s) and moz-action:keyword and moz-action:remotetab.
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review164634

Same question as the other bug, do we think the existing speculative test is enough? Shouldn't we have a test for this as well?

::: browser/base/content/urlbarBindings.xml:2306
(Diff revision 3)
> +        // on the popup.
> +        let item = event.originalTarget;
> +        while (item && item.localName != "richlistitem") {
> +          item = item.parentNode;
> +        }
> +        if (!item) {

This sounds expensive, maybe we could just check this.selectedIndex != -1?
If that doesn't work we can evaluate alternatives.
In the worst case the loop should break at the popup...

::: browser/base/content/urlbarBindings.xml:2310
(Diff revision 3)
> +        }
> +        if (!item) {
> +          return;
> +        }
> +
> +        let controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);

this.input.controller should do.

::: browser/base/content/urlbarBindings.xml:2311
(Diff revision 3)
> +        if (!item) {
> +          return;
> +        }
> +
> +        let controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);
> +        let url = controller.getFinalCompleteValueAt(this.selectedIndex);

Actually, we have a "bug" for which we may be showing results from the previous search, while a new one is running. I guess a user could be fast enough to mouse down on one of those ghost entries.
We should probably check that controller.matchCount > this.selectedIndex, in addition to the previous check.

::: browser/base/content/urlbarBindings.xml:2328
(Diff revision 3)
> +          // Where PARAMS is a JSON encoded object.
> +          const MOZ_ACTION_REGEX = /^moz-action:([^,]+),(.*)$/;
> +          if (!MOZ_ACTION_REGEX.test(url))
> +            return;
> +
> +          let params = JSON.parse(url.match(MOZ_ACTION_REGEX)[2]);

it would be better to use _parseActionUrl here, than to create your own parser.
Attachment #8885660 - Flags: review?(mak77)
Blocks: 481503
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review164634

> This sounds expensive, maybe we could just check this.selectedIndex != -1?
> If that doesn't work we can evaluate alternatives.
> In the worst case the loop should break at the popup...

Unfortunatelly it can't be determined by this.selectedIndex != -1 because this.selectedIndex is most likely to be init as 0 when the first result is a heuristic one in `onResultsAdded()`. I tried to avoid traversing tree back by trying other conditions, but failed to find one which can perfectly distiguish the case like clicking on one-off search buttons or the empty space beside one-off buttons from clicking on the richlistitems. Let me know if you have good idea.
FWIW, we also do the traverse back on mouseup in autocomplete.xml, but that case is a bit different since we bind mouseup event on richlistbox instead of the popup, which one-off search area isn't included.

> it would be better to use _parseActionUrl here, than to create your own parser.

I intended to not us `_parseActionUrl` because it did more than what I need, such as parsing displyUrl, the `losslessDecodeURI()` looks heavy (or it may not?). I just copied part of the parsing logic from this function, but I am fine to use it.
I'm not sure how to manipulate Places data to get moz-action:keyword or moz-action:remotetab items showing on the popup for my test case. I've seen other test examples that directly assign "moz-action:" string to gURLBar.value, but I don't think that fits into my scenario. :mak, do you have any idea to test this part of the code?
(In reply to Evelyn Hung [:evelyn] from comment #13)
> I'm not sure how to manipulate Places data to get moz-action:keyword or
> moz-action:remotetab items showing on the popup for my test case.

I don't think remotetab is trivial and it may be more work than the benefit.
for keyword, you can just use PlacesUtils.keywords.insert and PlacesUtils.keywords.remove, then typing "yourkw somework" should return a keyword entry. Note it will always be the heuristic result.
Off-hand, I doubt any keyword user would use the mouse to select it, it's a very compelling keyboard behavior. Indeed I think it's not worth supporting the keyword case for mousedown.
Tests look good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=88b540c76e6352767d11d25fefea4b0c9202a429&selectedJob=119516956

There is a lint error though. I will fix that before check-in.
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review168738

Almost ready, I would still like you to check a couple things regarding that DOM walking. I will be quick on the next pass, since I'm on PTO from Friday (but in the worst case you can ask :adw)

::: browser/base/content/test/urlbar/browser_urlbar_search_speculative_connect.js:10
(Diff revision 5)
>  "use strict";
>  
>  // This test ensures that we setup a speculative network
>  // connection for autoFilled values.
>  
>  let {HttpServer} = Cu.import("resource://testing-common/httpd.js", {});

no more needed?

::: browser/base/content/test/urlbar/browser_urlbar_search_speculative_connect_engine.js:10
(Diff revision 5)
>  "use strict";
>  
>  // This test ensures that we setup a speculative network connection to
>  // current search engine if the first result is 'searchengine'.
>  
>  let {HttpServer} = Cu.import("resource://testing-common/httpd.js", {});

no more needed?

::: browser/base/content/test/urlbar/browser_urlbar_search_speculative_connect_mousedown.js:10
(Diff revision 5)
> +"use strict";
> +
> +// This test ensures that we setup a speculative network connection to
> +// the site in mousedown event before the http request happens(in mouseup).
> +
> +let {HttpServer} = Cu.import("resource://testing-common/httpd.js", {});

no more needed?

::: browser/base/content/urlbarBindings.xml:2321
(Diff revision 5)
>          // This is copied from the mousedown handler in
>          // browser-search-autocomplete-result-popup, which apparently had a
>          // similar problem.
>          event.preventDefault();
> +
> +        // Respect to the user's setting

nit: I don't think this comment has much value, the code is readable enough.

::: browser/base/content/urlbarBindings.xml:2332
(Diff revision 5)
> +          return;
> +        }
> +        // Ensure the user is clicking on an url instead of other buttons
> +        // on the popup.
> +        let item = event.originalTarget;
> +        while (item && item.localName != "richlistitem") {

my fear is that if we should ever add a margin around richlistitems, or if this handles a mousedown out of the richlistbox then this will walk up to the window, and then to null, since it won't hit a richlistitem. That's expensive.
maybe we could somehow check event.target? What does it point to?

If we keep the loop, I'd suggest to stop it at the menupopup:
while (item && item.localName != "richlistitem" && item != this) (untested)
and then:
if (!item || item.localName != "richlistitem")

I'd probably also s/item/elt/ because the "item" word is a bit over-used in Places code.

Btw, I just found this.richlistbox.mousedOverIndex that looks interesting, could you please try if that'd work for us here?
Attachment #8885660 - Flags: review?(mak77)
In case I'm away when you have the patch ready, :adw can review, I'm still around today and tomorrow (just ping me on IRC for requests).
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review168738

> my fear is that if we should ever add a margin around richlistitems, or if this handles a mousedown out of the richlistbox then this will walk up to the window, and then to null, since it won't hit a richlistitem. That's expensive.
> maybe we could somehow check event.target? What does it point to?
> 
> If we keep the loop, I'd suggest to stop it at the menupopup:
> while (item && item.localName != "richlistitem" && item != this) (untested)
> and then:
> if (!item || item.localName != "richlistitem")
> 
> I'd probably also s/item/elt/ because the "item" word is a bit over-used in Places code.
> 
> Btw, I just found this.richlistbox.mousedOverIndex that looks interesting, could you please try if that'd work for us here?

this.richlistbox.mousedOverIndex isn't helpful in this case because it points to the (visually) last popup url item when you are clicking on the one-off search area. So it doesn't offer any useful information to help the judgement.

Therefore I still need the loop but stop it when we hit the event.target, which is the `this` object like you suggested. Thanks!
Comment on attachment 8885660 [details]
Bug 1355451 - Speculative connect to websites on mousedown on awesomebar item.

https://reviewboard.mozilla.org/r/156512/#review169724

::: browser/base/content/urlbarBindings.xml:2339
(Diff revision 6)
> +        if (!elt || elt.localName != "richlistitem") {
> +          return;
> +        }
> +        // The user might click on a ghost entry which was removed because of
> +        // the coming new results.
> +        if (this.input.mController.matchCount <= this.selectedIndex) {

this.input.controller should just work (IIRC controller is just a setter around mController)

::: browser/base/content/urlbarBindings.xml:2343
(Diff revision 6)
> +        // the coming new results.
> +        if (this.input.mController.matchCount <= this.selectedIndex) {
> +          return;
> +        }
> +
> +        let url = this.input.mController.getFinalCompleteValueAt(this.selectedIndex);

ditto
Attachment #8885660 - Flags: review+
sorry, I meant a getter, not a setter!
Pushed by ehung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/742dfaf7631d
Speculative connect to websites on mousedown on awesomebar item. r=mak
Backed out for eslint failure at urlbarBindings.xml:2331: multiple spaces before elt:

https://hg.mozilla.org/integration/autoland/rev/95e234ed81f5baa88700f8b423591fb1724788e3

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=120724997&repo=autoland

> TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/base/content/urlbarBindings.xml:2331:58 | Multiple spaces found before 'elt'. (no-multi-spaces)
Flags: needinfo?(ehung)
Pushed by ehung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a35f3b534eb9
Speculative connect to websites on mousedown on awesomebar item. r=mak
Sorry about that! I just fix the lint error, and reland it by pressing autoland.
Flags: needinfo?(ehung)
Sorry, I found a bug in my test case so it looked like failed but actually it didn't. I've fixed it and the try result looks good. I'm going to re-land the patch.

https://treeherder.mozilla.org/#/jobs?repo=try&author=ehung@mozilla.com&filter-tier=1
Flags: needinfo?(ehung)
Pushed by ehung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec99e3ba4e7a
Speculative connect to websites on mousedown on awesomebar item. r=mak
Backed out in https://hg.mozilla.org/integration/autoland/rev/bd0a13eefc6b for frequent failures like https://treeherder.mozilla.org/logviewer.html#?job_id=121151919&repo=autoland - there were a few on Linux and Mac debug, but by far your best chance of hitting it will be on Win8 debug, where it fails somewhere between 30% and 40% of the time.
Oh no~~ :'(
From the log, I guess this intermittent failure is caused by the delay of showing popup UI. I will investigate and try to fix it. Thanks!
Windows builds on try-server seem broken now. I will revisit this bug later.
:philor, Given the Windows build on try-server aren't stable, it's hard to debug this test failure. I wonder if we can disable this test and land the patch back since this is a performance improvement that we want to make it in Fx57. Also because we have other similar tests covered part of the code. If this is a solution, I will file another bug for fixing and enabling this broken test. Thanks.
Flags: needinfo?(philringnalda)
If you're talking about the permafailures on the buildbot-based Windows builds, that's expected, and you should just ignore those. The builds we use these days are built via Taskcluster and (at the moment) are prefixed with the "tc()" grouping when shown in Treeherder.
Flags: needinfo?(philringnalda) → needinfo?(ehung)
Interestingly, it seems unwilling to fail in https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6fe87077542f7b66f97785bb683ae03d906744f&group_state=expanded

The most likely two explanations for that are that the test depends on something that runs in another test which is currently (or, was currently at the time of that push) running in bc-3 but wasn't running in bc-4 at the time it landed, or that some test which was running in bc-4 at the time it landed doesn't clean something up, and the test fails to account for that possibility.
(In reply to Wes Kocher (:KWierso) from comment #37)
> If you're talking about the permafailures on the buildbot-based Windows
> builds, that's expected, and you should just ignore those. The builds we use
> these days are built via Taskcluster and (at the moment) are prefixed with
> the "tc()" grouping when shown in Treeherder.

Yes, I was saying the permafailures of the buildbot-based Windows builds. Thanks for clarifying for me.

(In reply to Phil Ringnalda (:philor) from comment #38)
> Interestingly, it seems unwilling to fail in
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=c6fe87077542f7b66f97785bb683ae03d906744f&group_state=e
> xpanded
> 
> The most likely two explanations for that are that the test depends on
> something that runs in another test which is currently (or, was currently at
> the time of that push) running in bc-3 but wasn't running in bc-4 at the
> time it landed, or that some test which was running in bc-4 at the time it
> landed doesn't clean something up, and the test fails to account for that
> possibility.

I read the test code again and I have no clue at all about the dependency. Since it can't be reproduced locally, I am going to mark checkin-needed and give it one more try.
Flags: needinfo?(ehung)
Keywords: checkin-needed
Autoland can't push from a closed MozReview request.
Flags: needinfo?(ehung)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4da627f785bc
Speculative connect to websites on mousedown on awesomebar item. r=mak
Flags: needinfo?(ehung)
Backed out for frequently failing browser_urlbar_search_speculative_connect_mousedown.js (especially on Linux debug):

https://hg.mozilla.org/integration/autoland/rev/7319c67162dbe07ffc1817a0685b6983f8f1c56b

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=123955452&repo=autoland
[task 2017-08-17T21:43:37.721058Z] 21:43:37     INFO - Entering test bound autofill_tests
[task 2017-08-17T21:43:37.728019Z] 21:43:37     INFO - Searching for 'ocal'
[task 2017-08-17T21:43:37.731262Z] 21:43:37     INFO - Buffered messages logged at 21:42:56
[task 2017-08-17T21:43:37.741133Z] 21:43:37     INFO - The value of the second item is http://localhost:44518/
[task 2017-08-17T21:43:37.751742Z] 21:43:37     INFO - TEST-PASS | browser/base/content/test/urlbar/browser_urlbar_search_speculative_connect_mousedown.js | The second item has the url we visited. - 
[task 2017-08-17T21:43:37.754458Z] 21:43:37     INFO - Buffered messages finished
[task 2017-08-17T21:43:37.759878Z] 21:43:37     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/urlbar/browser_urlbar_search_speculative_connect_mousedown.js | The second item is selected - Got 0, expected 1
[task 2017-08-17T21:43:37.764154Z] 21:43:37     INFO - Stack trace:
[task 2017-08-17T21:43:37.768166Z] 21:43:37     INFO - chrome://mochikit/content/browser-test.js:test_is:1000
[task 2017-08-17T21:43:37.777068Z] 21:43:37     INFO - chrome://mochitests/conte
Flags: needinfo?(ehung)
(In reply to Evelyn Hung [:evelyn] from comment #43)
> Comment on attachment 8885660 [details]
> Bug 1355451 - Speculative connect to websites on mousedown on awesomebar
> item.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/156512/diff/9-10/

I used debug build + rr to slow down my machine and then I was able to reproduce this test failure locally. It seemed to be caused by the UI delay like my initial thought in comment 34, so the mousedown event wasn't dispatched to the correct target on the popup menu. Therefore, I unfortunately have to add a setTimeout ans wait for 5 seconds in my test and see if this works. :(

Hi :mak, sorry for bothering you on the same bug again. Could you take a look of this test fix? Let me know if you have other idea. Thanks!
Flags: needinfo?(ehung) → needinfo?(mak77)
(In reply to Evelyn Hung [:evelyn] from comment #44)
> I used debug build + rr to slow down my machine and then I was able to
> reproduce this test failure locally. It seemed to be caused by the UI delay
> like my initial thought in comment 34, so the mousedown event wasn't
> dispatched to the correct target on the popup menu. Therefore, I
> unfortunately have to add a setTimeout ans wait for 5 seconds in my test and
> see if this works. :(

ah, hm, promiseAutocompleteResultPopup goes through promiseSearchComplete that goes through promisePopupShown. But this means at that point we're not ready yet. Maybe autocomplete is still animating its height and inserting results? The popup should be there since we got popupshown, it just looks like its contents are not ready.
I think it would be better if we could fix this at the promiseAutocompleteResultPopup level, than just in this bug, since that may fix other intermittents hitting the same problem. Plus, as you know, we are supposed to never use guessed timeouts.

We need a way to speed up populating and know when we are done. It may not be trivial though.
One thing we can try is setting a "dontanimate=true" attribute on the urlbar popup. It should then open and close quicker. It could be set and removed by promiseAutocompleteResultPopup itself.
Another possibility could be to use BrowserTestUtils.waitForCondition to check if the last match in the popup is visible.. maybe through getBoundingClientRect (not sure off-hand if there'd be a better way to check if tje richlistitem is really visible to the user)?

The last possibility, if we don't find a way to fix this at the promiseAutocompleteResultPopup level, could be to use BrowserTestUtils.waitForCondition to mousedown and wait for the SpeculativeConnection. It works similarly to the timeout, but it's a polling strategy at least.
Flags: needinfo?(mak77)
Thanks for the clue, mak. It seems setting "dontanimate=true" works.

So I found the intermittent error was caused by popup animation. The item was there already and its getBoundingClientRect returned correct position and weight/height. However, the popup itself was still animating its height. Therefore EventUtils.synthesizeMouse was hitting on the popup, not the richlistitem. It's also sad that `document.elementsFromPoint()` wouldn't see the popup UI (I don't know why) even when the popup is visually there, so I didn't know if the (x, y) hits on my target element.

Anyway, please take a look of the fix and let me know if anything missing. Thanks!
Flags: needinfo?(mak77)
Provided you got a green Try run out of this across the tier-1 platforms, it looks good to me.
Flags: needinfo?(mak77)
Pushed by ehung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae598fdd78c8
Speculative connect to websites on mousedown on awesomebar item. r=mak
https://hg.mozilla.org/mozilla-central/rev/ae598fdd78c8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Iteration: --- → 57.2 - Aug 29
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: