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)

defect

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.4 - May 1
Tracking Status
firefox55 --- verified
firefox56 --- verified

People

(Reporter: florian, Assigned: mak)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-performance][fxsearch])

Attachments

(3 files)

Attached video Screencast (deleted) —
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
Flags: qe-verify?
Priority: -- → P2
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.
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
Status: NEW → ASSIGNED
Priority: P2 → P1
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.
I split the backspace problem to bug 1360506.
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.
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+
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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.4 - May 1
No longer blocks: photon-performance-triage
Flags: qe-verify? → qe-verify+
QA Contact: adrian.florinescu
still, I found this issue on Latest Nightly-- Build ID:20170520030204 windows7 32bit The global icon is flicker
(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.
Attached video icon-flickr-nightly-55.mp4 (deleted) —
(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
seen the video, this is not the right bug, bug 1363621 is the right one, because you are typing a url.
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: