Closed
Bug 1360279
Opened 8 years ago
Closed 8 years ago
Typing in the location bar after a keyword causes an icon to flicker
Categories
(Firefox :: Address Bar, defect, P1)
Firefox
Address Bar
Tracking
()
People
(Reporter: florian, Assigned: mak)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-performance][fxsearch])
Attachments
(3 files)
See the attached screencast.
While typing, the globe icon between the glass icon and the 'bugzilla.mozilla.org' hostname flickers.
While backspacing, the whole awesomebar panel blinks.
I don't have a profile of the exact interaction in the video (too bad I would be curious to see what caused the big pause while backspacing), but here is a profile of a similar interaction: https://perfht.ml/2oN59fq
Updated•8 years ago
|
Flags: qe-verify?
Priority: -- → P2
Assignee | ||
Comment 1•8 years ago
|
||
The problem is that before hi-res icons, all of these entries had the same icon, the default one. Instead now potentially every url we generate may have an icon, so the previous image is replaced with a new one. We have a similar problem on the other places views. I'm not sure how to fix this, most of the xul views will remove the previous image before the new one is fully loaded. for the keyword case we could probably workaround the problem by not replacing the icon at every string change.
Or we should find a more general solution, that I don't know (maybe we could use a transition when the image changes so that it stays visible for a while more? maybe we could change widgets to not replace the image until the new one is loaded?)
The backspace flicker is likely there from longer time, it's probably due to the fact when there's 0 results we hide the popup immediately. Should be fixable.
Assignee | ||
Comment 2•8 years ago
|
||
thinking more about this, for keywords we should not use the full url, we should instead just use the domain.
that means changing this line:
http://searchfox.org/mozilla-central/rev/ce5ccb6a8ca803271c946ccb8b43b7e7d8b64e7a/toolkit/components/places/UnifiedComplete.js#1384
I'll look into this tomorrow.
the backspace problem should be handled in a separate bug since it's totally unrelated to the icon one.
Assignee: nobody → mak77
Updated•8 years ago
|
Status: NEW → ASSIGNED
Priority: P2 → P1
Assignee | ||
Comment 3•8 years ago
|
||
and re: the backspace problem, it may be enough to set minresultsforpopup="0" on the urlbar input field (TBV). It seems to make sense now that we have the heuristic result and one-offs. we could have to monkeypatch the history dropdown case, that is the only one where we don't have an heuristic result.
Assignee | ||
Comment 4•8 years ago
|
||
I split the backspace problem to bug 1360506.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•8 years ago
|
||
This should solve the flicker, I'm using the non-parsed url instead of the parsed url, it doesn't really matter for favicons since the only possibility is that we have a root icon for that domain in the db. This way the url is always the same and we just keep using the image cache.
Reporter | ||
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8862788 [details]
Bug 1360279 - Typing in the location bar after a keyword causes the heuristic result icon to flicker.
https://reviewboard.mozilla.org/r/134674/#review137636
Looks reasonable to me, thanks for getting to this so quickly! :-)
Other small optimization suggestions from a quick look at that file:
http://searchfox.org/mozilla-central/source/toolkit/components/places/UnifiedComplete.js#1371-1377
could be:
let style = (this._enableActions ? "action " : "") + "keyword";
let actionURL = PlacesUtils.mozActionURI("keyword", {
url,
input: this._originalSearchString,
postData,
});
let value = this._enableActions ? actionURL : url;
let style = "keyword";
let value = url;
if (this._enableActions) {
style = "action " + style;
value = PlacesUtils.mozActionURI("keyword", {
url,
input: this._originalSearchString,
postData,
});
}
And NetUtil.jsm could easily be avoided.
Attachment #8862788 -
Flags: review?(florian) → review+
Comment hidden (mozreview-request) |
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/6b28cb024e2f
Typing in the location bar after a keyword causes the heuristic result icon to flicker. r=florian
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•8 years ago
|
Iteration: --- → 55.4 - May 1
Reporter | ||
Updated•8 years ago
|
No longer blocks: photon-performance-triage
Reporter | ||
Updated•8 years ago
|
Blocks: photon-performance-triage
Updated•8 years ago
|
Flags: qe-verify? → qe-verify+
QA Contact: adrian.florinescu
Comment 11•8 years ago
|
||
still, I found this issue on Latest Nightly-- Build ID:20170520030204
windows7 32bit
The global icon is flicker
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Saheda Reza [:Antora] from comment #11)
> still, I found this issue on Latest Nightly-- Build ID:20170520030204
> windows7 32bit
> The global icon is flicker
Probably you are seeing bug 1363621, not this one.
Comment 13•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #12)
> Probably you are seeing bug 1363621, not this one.
Notice the globe icon (which is not favicon, the screencast has no favicon on newtab ), I guess this is the right bug.
Here is an attachment from latest nightly (details mentioned):
Version 55.0a1
Build ID 20170521030205
Update Channel nightly
User Agent Mozilla/5.0 (Windows NT 6.1; rv:55.0) Gecko/20100101 Firefox/55.0
Assignee | ||
Comment 14•8 years ago
|
||
seen the video, this is not the right bug, bug 1363621 is the right one, because you are typing a url.
Comment 15•7 years ago
|
||
Verified as fixed using the latest Nightly 56.0a1 (2017-07-12) and latest Beta 55.0b8 on Ubuntu 16.04, Mac OS X 10.12 and Windows 10 x64.
Updated•7 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•