Closed
Bug 1475589
Opened 6 years ago
Closed 6 years ago
formatValue in URL bar doesn't get re-run when dragging tab to new window
Categories
(Firefox :: Address Bar, defect, P2)
Firefox
Address Bar
Tracking
()
VERIFIED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | wontfix |
firefox61 | --- | wontfix |
firefox62 | --- | verified |
firefox63 | --- | verified |
People
(Reporter: Gijs, Assigned: florian)
References
Details
(Keywords: regression, Whiteboard: [fxsearch])
Attachments
(1 file)
(deleted),
patch
|
Gijs
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR:
1. open https://www.mozilla.org/
2. (note "mozilla.org" is highlighted in URL bar, everything else has lower opacity text)
3. open another tab in that window if this is the only tab
4. switch back to mozilla.org tab and drag it into its own window
ER:
URL formatting is the same as before
AR:
No more URL formatting
Bonus extra: if you drag the tab back to its original window, the formatting returns in that URL bar. Magic!
Reporter | ||
Comment 1•6 years ago
|
||
58 was also affected... but 52 wasn't.
status-firefox61:
--- → wontfix
status-firefox62:
--- → fix-optional
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → fix-optional
Keywords: regression,
regressionwindow-wanted
Reporter | ||
Comment 2•6 years ago
|
||
Seems this was caused by bug 1391704.
Florian, can you take a look? The URL bar formatting is supposed to help convey security information to the user so ideally we shouldn't break it. This may be related to some of the other textbox/focus regressions that have been recently filed against bug 1391704...
Updated•6 years ago
|
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #2)
> This may be related to some of the other textbox/focus regressions that have
> been recently filed against bug 1391704...
It's not related to bug 1459327 (I'm assuming this is what you had in mind).
The problem here is a bad interaction with the hacks I introduced in bug 1403648 that add the "focused" attribute by default on the urlbar and remove it later if we aren't sure the urlbar will actually be focused.
The formatValue code returns early if the urlbar is focused, and that's what's happening here.
My first idea for a trivial fix was to add gURLBar.formatValue(); at https://searchfox.org/mozilla-central/rev/d47c829065767b6f36d29303d650bffb7c4f94a3/browser/base/content/browser.js#1646 This works, but somehow the urlbar focus ring still flickers for a frame or so.
I think a better fix is to remove the "focused" attribute early in this case, because we know the urlbar won't be focused when we are adopting a tab.
Assignee: nobody → florian
Flags: needinfo?(florian)
Assignee | ||
Comment 5•6 years ago
|
||
Attachment #8995533 -
Flags: review?(gijskruitbosch+bugs)
Reporter | ||
Updated•6 years ago
|
Attachment #8995533 -
Flags: review?(gijskruitbosch+bugs) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15f7f56e614d
remove the focused attribute early on the urlbar when adopting a tab, to ensure the url gets properly formatted, r=Gijs.
Comment 7•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment 8•6 years ago
|
||
Doesn't seem impactful enough to worry about for ESR60, but did you want to nominate this for Beta consideration?
Flags: needinfo?(florian)
Assignee | ||
Comment 9•6 years ago
|
||
Comment on attachment 8995533 [details] [diff] [review]
Patch
(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> Doesn't seem impactful enough to worry about for ESR60, but did you want to
> nominate this for Beta consideration?
Given that this regressed in 57 and only got noticed about a month ago, I don't think there's any urgency to fix this, but on the other hand the fix is very low risk and as Gijs mentioned in comment 2, this is supposed to be a security feature, so it's better to not break it.
I'm adding a beta approval request, but I don't mind either way.
Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Broken coloration of the hostname in the urlbar of detached tabs.
[Is this code covered by automated tests?]: no.
[Has the fix been verified in Nightly?]: Doesn't look like it.
[Needs manual test from QE? If yes, steps to reproduce]: steps in comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no.
[Why is the change risky/not risky?]: Just removing the focus from the urlbar earlier, only in the specific case of detaching a tab.
[String changes made/needed]: none
Flags: needinfo?(florian)
Attachment #8995533 -
Flags: approval-mozilla-beta?
Comment on attachment 8995533 [details] [diff] [review]
Patch
Seems like a reasonably low risk fix. Let's uplift for beta 17.
Attachment #8995533 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Flags: qe-verify+
Comment 12•6 years ago
|
||
I could reproduce this issue (comment 0) on
Build ID 20180704003137
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
I tested this bug fix on following
Build ID 20180814100100
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
Version 62.0b17
Build ID 20180813190114
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:63.0) Gecko/20100101 Firefox/63.0
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:62.0) Gecko/20100101 Firefox/62.0
URL formatting is working as expected when moving from one window to another/or dragging back to original window.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•