Closed Bug 1457355 Opened 2 years ago Closed 6 months ago

New tab overwrites X11/Linux primary selection when location bar text is highlighted

Categories

(Firefox :: Address Bar, defect, P3)

60 Branch
Unspecified
Linux
defect

Tracking

()

RESOLVED FIXED
Firefox 70
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox59 --- unaffected
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

(Keywords: dataloss, regression)

Attachments

(2 files)

1. Run Firefox 60.0b14 in a new profile and load (for example)
   https://www.mozilla.org/en-US/firefox/nightly/firstrun/
2. Select "mozilla" in URL bar.
3. Click "Open a new tab" button.
4. Close new tab.
   Location bar text is highlighted (a variation of bug 1393059).
5. Click "Open a new tab" button.
6. Click middle mouse button to insert primary selection,
   in location bar or elsewhere.

Expected new text: "mozilla".
Actual new text: "https://www.mozilla.org/en-US/firefox/nightly/firstrun/"

Regression from https://hg.mozilla.org/integration/autoland/rev/562966f195b0
Slightly different steps are required to reproduce on recent Nightlies, due to
a different variation of bug 1393059.  These steps reproduce on both Nightly
and 60.0b14:

1. Run browser in a new profile and load (if not already shown)
   https://www.mozilla.org/en-US/firefox/nightly/firstrun/
2. Select "mozilla" in URL bar.
3. (not required)
4. Select text in another app. e.g. terminal, or another web browser.
5. Click "Open a new tab" button.
6. Click middle mouse button to insert primary selection,
   in location bar or elsewhere.

Expected new text: (from other app in step 3)
Actual new text: "mozilla".
No clue how bug 1442651 would have caused this, but this sounds like an Address Bar bug. FWIW, we have code that explicitly writes to the primary selection here:

https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/browser/base/content/urlbarBindings.xml#1732-1750

Maybe the isHandlingUserInput check is broken?
Component: Tabbed Browser → Address Bar
Priority: -- → P3
Duplicate of this bug: 1462508
Duplicate of this bug: 1487394
Summary: new tab overwrites primary selection when location bar text is highlighted → New tab overwrites X11/Linux primary selection when location bar text is highlighted

This reproduces reliably on new tab.
It sometimes reproduces even when switching to an existing new tab.
It doesn't seem to happen when opening about:preferences or about:addons in a new tab.

The unexpected modification to the primary selection on new tab happens with this call stack on revision adc59d50adf8:

0 _on_select(event = [object Event]) ["resource:///modules/UrlbarInput.jsm":1425:20]
this = [object Object]
1 handleEvent(event = [object Event]) ["resource:///modules/UrlbarInput.jsm":333:22]
this = [object Object]
2 select() ["chrome://global/content/bindings/textbox.xml":107:26]
this = [object XULElement]
3 UrlbarInput/thismethod ["resource:///modules/UrlbarInput.jsm":115:35]
4 focusAndSelectUrlBar() ["chrome://browser/content/browser.js":2428:10]
this = [object ChromeWindow]
5 _adjustFocusAfterTabSwitch(newTab = [object XULElement]) ["chrome://browser/content/tabbrowser.js":1186:8]
this = [object Object]
6 updateDisplay() ["resource:///modules/AsyncTabSwitcher.jsm":440:26]
this = [object Object]
7 postActions() ["resource:///modules/AsyncTabSwitcher.jsm":618:9]
this = [object Object]
8 queueUnload(unloadTimeout = 300) ["resource:///modules/AsyncTabSwitcher.jsm":1026:9]
this = [object Object]
9 requestTab(tab = [object XULElement]) ["resource:///modules/AsyncTabSwitcher.jsm":1015:9]
this = [object Object]
10 updateCurrentBrowser() ["chrome://browser/content/tabbrowser.js":917:28]
this = [object Object]
11 _setupEventListeners/<(event = [object Event]) ["chrome://browser/content/tabbrowser.js":4509:13]
12 set selectedIndex(val = 1) ["chrome://global/content/elements/tabbox.js":193:11]
this = [object XULElement]
13 set selectedPanel(val = [object XULElement]) ["chrome://global/content/elements/tabbox.js":207:4]
this = [object XULElement]
14 set selectedIndex(val = 1) ["chrome://global/content/elements/tabbox.js":532:8]
this = [object XULElement]
15 set selectedItem(val = [object XULElement]) ["chrome://global/content/elements/tabbox.js":551:32]
this = [object XULElement]
16 set selectedTab(val = [object XULElement]) ["chrome://global/content/elements/tabbox.js":86:8]
this = [object XULElement]
17 set selectedTab(val = [object XULElement]) ["chrome://browser/content/tabbrowser.js":261:4]
this = [object Object]
18 loadOneTab(aURI = "about:newtab"

_gBrowser.updateCurrentBrowser calls requestTab before setting _selectedTab and "Update the URL bar".

The location bar contents are subsequently updated at

0 _setValue(val = "", allowTrim = true) ["resource:///modules/UrlbarInput.jsm":767:4]
<failed to get 'this' value>
1 set value(val = "") ["resource:///modules/UrlbarInput.jsm":749:16]
<failed to get 'this' value>
2 URLBarSetURI( <Failed to get argument while inspecting stack frame>
updatePopupNotifications = true) ["chrome://browser/content/browser.js":2847:2]
<failed to get 'this' value>
3 onLocationChange( <Failed to get argument while inspecting stack frame>
aRequest = null <Failed to get argument while inspecting stack frame>
, aFlags = 0, aIsSimulated = true) ["chrome://browser/content/browser.js":4895:6]
<failed to get 'this' value>
4 callListeners( <Failed to get argument while inspecting stack frame>
<Failed to get argument while inspecting stack frame>
) ["chrome://browser/content/tabbrowser.js":742:28]
<failed to get 'this' value>
5 _callProgressListeners(aBrowser = null, aMethod = "onLocationChange" <Failed to get argument while inspecting stack frame>
, aCallGlobalListeners = true, aCallTabsListeners = false) ["chrome://browser/content/tabbrowser.js":755:6]
<failed to get 'this' value>
6 updateCurrentBrowser() ["chrome://browser/content/tabbrowser.js":965:9]
<failed to get 'this' value>

This ordering dates back to at least "Tab switch refactoring".
https://hg.mozilla.org/mozilla-central/rev/564a9e11d9ec89b0d9a6e2ea7e93cd531c9a468d
I don't know whether this was the ordering at the time of https://hg.mozilla.org/integration/autoland/rev/562966f195b0 triggering this regression. I haven't identified how that triggered this regression.

Assignee: nobody → karlt
Status: NEW → ASSIGNED

The intention of UrlbarInput._on_select() is to adjust the primary after it has been set (differently) by the editor. The editor does not set the primary during HTMLInputElement::Select() as used by focusAndSelectUrlBar(), and similarly _on_select() should not adjust the primary in this situation.

I also investigated changing focusAndSelectUrlBar() to merely focus and not select the urlbar, but selection is expected at least on keyboard shortcuts to directly focus the location bar.

This suppresses modification of the primary selection when opening a new tab
as well as when a keyboard shortcut is used to directly focus the location
bar.

Attachment #9076706 - Attachment description: Bug 1457355 don't change primary selection during focusAndSelectUrlBar() r?dao → bug 1457355 don't change primary selection during UrlbarInput.select() r?dao

@karlt thanks so much for working on this! I'll be much happier once this is fixed.

Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/435cbd9fb896
don't change primary selection during UrlbarInput.select() r=dao
https://hg.mozilla.org/integration/autoland/rev/3cf34ed0ac83
test that primary selection is unaffected by opening new tab r=dao
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Is this something you wanted to nominate for Beta approval or can this ride the trains?

Flags: needinfo?(karlt)

This has existed long enough that I think there is no urgency to uplift, thanks. 69 is also not an ESR so no benefit there.
There would be a small risk to uplift with some of the refactoring that has been happening, and so safer to ride trains.

Flags: needinfo?(karlt)
You need to log in before you can comment on or make changes to this bug.