Closed
Bug 1513469
Opened 5 years ago
Closed 5 years ago
First entry in the urlbar dropdown is no longer highlighted by default
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
RESOLVED
FIXED
Firefox 66
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | --- | unaffected |
firefox66 | + | fixed |
People
(Reporter: bruce.bugz, Assigned: mak)
References
(Depends on 1 open bug)
Details
(Keywords: regression, Whiteboard: [fxsearch])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0 Steps to reproduce: Type `test` in the address bar without quotes. Actual results: The `[lens] test` line did not have the usual blue background. If I press down-arrow, followed by up-arrow, it still does not get the blue background. Expected results: The `[lens] test` line should have a blue background.
Assignee | ||
Comment 1•5 years ago
|
||
It looks like something regressed this very recently
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•5 years ago
|
Priority: -- → P1
Whiteboard: [fxsearch]
Assignee | ||
Comment 2•5 years ago
|
||
Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=53fd96ca5aa4298054f581ca846ea2cccbe76085&tochange=6915aee134cb2b579b67e7718579c7a59b15e1d8
Assignee | ||
Comment 3•5 years ago
|
||
off-hand it looks like it may be due to bug 1492326. Neil, any idea?
Blocks: 1492326
Flags: needinfo?(enndeakin)
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Keywords: regression
Comment 4•5 years ago
|
||
It looks like some code is trying to retrieve richlistbox.itemChildren when that first item has not yet been initialized (has no binding). The old version of "itemChildren" would skip that item, whereas the new version will just return all richlistitems. The latter is probably more correct considering that "itemChildren" should return its children. This is a stacktrace of the caller: chrome://global/content/bindings/richlistbox.xml 601 get_itemChildren chrome://global/content/bindings/richlistbox.xml 221 getItemAtIndex chrome://global/content/bindings/richlistbox.xml 163 set_selectedIndex chrome://global/content/bindings/autocomplete.xml 760 set_selectedIndex chrome://browser/content/urlbarBindings.xml 2483 onResultsAdded chrome://global/content/bindings/autocomplete.xml 1059 _appendCurrentResult chrome://global/content/bindings/autocomplete.xml 864 _invalidate chrome://browser/content/urlbarBindings.xml 2194 _openAutocompletePopup chrome://browser/content/urlbarBindings.xml 2107 openAutocompletePopup chrome://global/content/bindings/autocomplete.xml 344 openPopup chrome://global/content/bindings/autocomplete.xml 90 set_popupOpen
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 5•5 years ago
|
||
I'm not sure I understand, we set the selection to the item at index 0 (see in urlbarbindings this.input.controller.setInitiallySelectedIndex(0)), after that item has been appended to the dom. What do you mean with "the old version would skip that item", the item at 0 was properly selected in the old version, if the new version returns all the items, it should still select the item at 0. Are you saying we set the selection on a different item than the one we expect to be at index 0?
Comment 6•5 years ago
|
||
With the old code, it looks like setInitiallySelectedIndex initially fails, as the first item has a node, but is not present in itemChildren, as described above. setInitiallySelectedIndex then gets called again on a later call to invalidate() and is successful in assigning to the first index. With the new code, the first call to setInitiallySelectedIndex succeeds, but because the binding for that item isn't yet present, the selected attribute/property never gets assigned. I'm not familiar enough with the mechanism that is calling setInitiallySelectedIndex to know if it should/should not be reseting something that causes it to get called again, or perhaps the richlistbox should handle the element as not yet selected.
Assignee | ||
Comment 7•5 years ago
|
||
Let me see if I got this correctly. We try to set richlistitem.selected when the binding is not attached yet, due to this we end up setting an expando on it that will prevent the "selected" property of the binding from working properly once attached. Indeed the item even later is basically stuck and can't be selected anymore. the only solution I found so far is to delay onResultsAdded, so that the binding is attached by the time we try to set selection. This is not great though (may be ok-ish temporarily, considered we are moving off of this code), the richlistbox should not break its children, since we have a richlistbox, we should be able to call selectedIndex at any time, without it breaking its internals.
Assignee | ||
Comment 8•5 years ago
|
||
Or, we could remove the "selected" property and just use plain attributes. The property seems to be used only inside richlistbox anyway
Assignee | ||
Comment 9•5 years ago
|
||
Hm but it implements nsIDOMXULSelectControlItemElement :(
Assignee | ||
Comment 10•5 years ago
|
||
I'll test a patch delaying onResultsAdded on Try, if you have better ideas to fix richlistbox, they are welcome, but we need to address this regression sooner than later.
Comment 11•5 years ago
|
||
We can also just back out the one-line itemChildren change from richlistbox.xml until we find a better fix.
Assignee | ||
Comment 12•5 years ago
|
||
We must delay setting the selected index, otherwise we may end dealing with richlistitems without an applied binding, and the richlistbox may break them permanently by setting a "selected" expando.
Assignee | ||
Comment 13•5 years ago
|
||
let's try with this location bar fix, we should probably still track the richlistbox issue apart in toolkit, because I still don't think it's ok that a binding can break its internals.
Updated•5 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Comment 14•5 years ago
|
||
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/974da7ec60a9 First entry in the urlbar dropdown is no longer highlighted by default. r=adw
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/974da7ec60a9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Updated•5 years ago
|
status-firefox64:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Comment 16•4 years ago
|
||
I have reproduced this bug with Nightly 66.0a1 (2018-12-12) in Windows 10, 64 Bit. This bug's fix is verified with latest Nightly!
Build ID : 20190120213632
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0
QA Whiteboard: [bugday-20190116]
Updated•4 years ago
|
QA Whiteboard: [bugday-20190116] → [bugday-20190116] [good first verify]
You need to log in
before you can comment on or make changes to this bug.
Description
•