Reproducible on: Firefox 46.0a1 and Firefox 45.0a2 across all platforms STR 1.Launch Firefox with a clean profile. 2.Create a Firefox account and confirm it. 3.Log in using the recently created account. 4.Open the Panel Menu -> Click on 'Synced tabs' icon. 5.Click on “Firefox for Android” or “Firefox for iOS” link. ER Once the “Firefox for Android” or “Firefox for iOS” page is opened the Synced Tabs panel disappears. AR The Synced Tabs panel remains displayed while the “Firefox for Android” or “Firefox for iOS” page is opened. See screenshot: http://i.imgur.com/jjcuLX4.jpg Additional notes: - Reproducible on: Firefox 46.0a1 (2016-01-07) and Firefox 45.0a2 (2016-01-07) using Windows 10 64-bit, Mac 10.10.5 and Ubuntu 14.04 32-bit.

The issue here is that the text-link widget is responsible for opening the link, but it obviously doesn't know anything about the panel. There's also bug 1237942, which reports that the links can't be clicked using other mouse buttons - and while that bug mentions the test-link widget probably can be changed to support other buttons, that still leaves us with the fact the panel would remain open and no obvious way to intercept or detect that. So this patch attempts to kill both birds with one stone: * An explicit click handler that handles all buttons and explicitly opens the link in a tab (ie, thereby fixing bug 1237942) * That handler also closes the panel UI (ie, fixing this bug) I've also attempted to handle a RETURN key opening the link, but given the panel UI isn't accessible it's tricky to manually test this aspect - so I'm not sure if it is worthwhile. However, I think I can add tests for both clicks and RETURN - I just haven't done that until I get feedback on the general approach.

Comment on attachment 8709749 [details] [diff] [review] 0002-Bug-1237945-ensure-panel-is-closed-when-mobile-promo.patch Sadly, I wouldn't worry too much about the keyboard case right now... we really need to sort out the keyboard nav in a comprehensive fashion at some point. Right now there isn't really a lot we can do about it, and the keyboard event handler doesn't really help because there's no reasonable way you can focus the links anyway. Otherwise, this looks OK to me generally.

Thanks for the typically prompt feedback! (In reply to :Gijs Kruitbosch from comment #2) > Sadly, I wouldn't worry too much about the keyboard case right now... Fair enough - I've made this change, and also another small change so the final URL is built in the click handler rather than directly in onCreated - this way the test can tweak the preference after the widget is built and have the new pref value used.

Comment on attachment 8710268 [details] [diff] [review] 0002-Bug-1237945-ensure-panel-is-closed-when-mobile-promo.patch Review of attachment 8710268 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/customizableui/test/browser_967000_button_sync.js @@ +36,5 @@ > + event.initMouseEvent("click", true, true, window, > + 0, 0, 0, 0, 0, > + ctrlKeyArg, altKeyArg, shiftKeyArg, metaKeyArg, > + buttonArg, null); > + target.dispatchEvent(event); Why can't we use EventUtils? @@ +169,5 @@ > + for (let link of links) { > + for (let button = 0; button < 3; button++) { > + simulateClick({ button }, link); > + // the panel should have been closed. > + ok(!isPanelUIOpen(), "click closed the panel"); I don't think a right-click here should close the panel, or open the links...

(In reply to :Gijs Kruitbosch from comment #4) > Why can't we use EventUtils? We can - I didn't think to look there :( > I don't think a right-click here should close the panel, or open the links... Fair enough - fix this too - right-click is now ignored. Thanks!

Comment on attachment 8710864 [details] [diff] [review] 0002-Bug-1237945-ensure-panel-is-closed-when-mobile-promo.patch Review of attachment 8710864 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, thanks! PS: can you find/file a followup for the middle-click-on-text-link thing? That seems very fixable...

Backed out in https://hg.mozilla.org/integration/fx-team/rev/4a57702113ed for frequent Linux32 debug e10s and ASan e10s https://treeherder.mozilla.org/logviewer.html#?job_id=6790815&repo=fx-team and https://treeherder.mozilla.org/logviewer.html#?job_id=6790814&repo=fx-team

(In reply to :Gijs Kruitbosch from comment #6) > PS: can you find/file a followup for the middle-click-on-text-link thing? > That seems very fixable... Done - bug 1246082. (In reply to Phil Ringnalda (:philor) from comment #8) > Backed out in https://hg.mozilla.org/integration/fx-team/rev/4a57702113ed > for frequent Linux32 debug e10s and ASan e10s Lots of try runs later, it seems this was a simple timeout - relanding with requestLongerTimeout(2) in the test.

I was able to reproduce this issue on Firefox 46.0a1 using Windows 10 64-bit. Verified fixed on Firefox 47.0a1 (2016-02-28) under Windows 10 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.11. The Synced Tabs panel is successfully closed while opening the “Firefox for Android” or “Firefox for iOS” link.