Closed
Bug 1264988
Opened 9 years ago
Closed 7 years ago
Scrollbar appears for a moment in the new Awesomebar Resultlist
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox56 | --- | verified |
People
(Reporter: mehmet.sahin, Assigned: mak)
References
Details
(Whiteboard: [fxsearch] [photon-performance])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160415030231
Steps to reproduce:
Firefox Nightly 48.0a1 (2016-04-15)
Mac OS 10.11.4
STR:
1.) type something into the Location Bar, so that only one item is to see in the Resultlist
2.) Delete some letters quickly
Actual results:
The scrolbar appears for a moment.
Expected results:
The scrollbar should not appear.
Assignee | ||
Comment 1•9 years ago
|
||
yep, that's a little bit noisy... We may maybe use overflow: hidden... though users could still set browser.urlbar.maxRichResults and we should not break their urlbar.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Whiteboard: [fxsearch]
OS: Mac OS X → All
Hardware: Unspecified → All
Summary: [OSX] Scrollbar appears for a moment in the new Awesomebar Resultlist → Scrollbar appears for a moment in the new Awesomebar Resultlist
Marco's comment about users with a different value for browser.urlbar.maxRichResults is alluding to values which are high enough that the results can extend beyond the results pane's max-height.
Perhaps the solution is to set overflow: hidden during the resize and unset it afterwords.
In linux (Fedora) also happens. But the scrollbar it is permanent if the result box it is on his max size
hiding a couple of items.
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to jorrete from comment #5)
> In linux (Fedora) also happens. But the scrollbar it is permanent if the
> result box it is on his max size
> hiding a couple of items.
did you change browser.urlbar.maxRichResults ?
Which version of Firefox?
Assignee | ||
Updated•8 years ago
|
Priority: P2 → P3
Could this be taken into account for Photon perceived performance ?
Flags: needinfo?(mak77)
Assignee | ||
Comment 8•8 years ago
|
||
yep.
Blocks: photon-performance-triage
Flags: needinfo?(mak77)
Whiteboard: [fxsearch] → [fxsearch][photon-performance]
Updated•8 years ago
|
Priority: P3 → P2
Updated•8 years ago
|
Flags: qe-verify?
Updated•8 years ago
|
Flags: qe-verify? → qe-verify+
Updated•8 years ago
|
QA Contact: adrian.florinescu
Updated•8 years ago
|
Blocks: photon-perf-awesomebar
Updated•8 years ago
|
No longer blocks: photon-performance-triage
Updated•8 years ago
|
Priority: P2 → P3
Whiteboard: [fxsearch][photon-performance] → [fxsearch] [reserve-photon-performance]
Assignee | ||
Updated•8 years ago
|
Priority: P3 → P2
Assignee | ||
Comment 9•8 years ago
|
||
oh hm, sorry I didn't see Marco changed the priority.
Priority: P2 → P3
Comment 10•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #9)
> oh hm, sorry I didn't see Marco changed the priority.
Marco changed the priority (along with the priority of lots of other photon perf bugs) because we decided this bug goes on the 'reserve' for photon-perf. The reason for putting the bug in the photon reserve in this case is that we expect the fxsearch team to handle it... so feel free to set the priority to whatever makes sense for you (especially if there are chances you could fix it, which I would really appreciate ;-)).
Assignee | ||
Comment 11•8 years ago
|
||
Ok, I didn't want to cause issues to your tracking, I think we should fix this for the speed perception (and showing the scrollbar may even have a cost).
Priority: P3 → P2
Comment 12•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #11)
> (and showing the scrollbar may even have a cost).
Unless I missed a newer patch, we added a fast code path that skips a sync reflow only when there's no scrollbar, because we assumed it was always the case for the awesomebar... so I'm afraid the cost here is a sync reflow.
Comment 13•8 years ago
|
||
But the flickering perception is enough for this to be worth fixing anyway.
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] (away until Monday May 29th) from comment #12)
> Unless I missed a newer patch, we added a fast code path that skips a sync
> reflow only when there's no scrollbar, because we assumed it was always the
> case for the awesomebar... so I'm afraid the cost here is a sync reflow.
No, what was causing the reflow was a call to ensureItemIsVisible, and to avoid that we checked if we could never have a scrollbar in a theoretical way, by comparing maximum number of entries. Since we know that the urlbar doesn't want a scrollbar we always skip the reflow because we don't care.
Having a scrollbar or not *in reality* doesn't have any effect on that, the cost I was speaking about is that layout may have to do more computation to show a scrollbar rather than an overflow:hidden frame (but I don't know if that's true).
Updated•8 years ago
|
Whiteboard: [fxsearch] [reserve-photon-performance] → [fxsearch] [photon-performance]
Assignee | ||
Comment 15•7 years ago
|
||
I may look at this next week
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Updated•7 years ago
|
Priority: P2 → P1
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
there is another alternative, that is to tamporarily set overflow hidden when we modify the contents, and restore to auto once done. It should be done somewhere between _appendCurrentResult and adjustHeight in autocomplete.xml to completely avoid the scrollbar. I didn't try since it sounds far more work and complication for a small benefit of a single consumer. If we'd ever find the problem is also annoying in other consumers of the richlistbox autocomplete, we may have to look into that.
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8889407 [details]
Bug 1264988 - Scrollbar flickers in the Awesomebar results list.
https://reviewboard.mozilla.org/r/160440/#review165724
A solution specific to this popup sounds good to me.
::: browser/base/content/urlbarBindings.xml:187
(Diff revision 1)
>
> <!--
> + Since we never want scrollbars, we always use the maxResults value.
> + -->
> + <property name="maxRows"
> + onget="return this.popup.maxResults || Services.prefs.getIntPref(`browser.urlbar.maxRichResults`);"/>
Doesn't popup.maxResults always have a value?
Attachment #8889407 -
Flags: review?(paolo.mozmail) → review+
Assignee | ||
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8889407 [details]
Bug 1264988 - Scrollbar flickers in the Awesomebar results list.
https://reviewboard.mozilla.org/r/160440/#review165784
::: browser/base/content/urlbarBindings.xml:187
(Diff revision 1)
>
> <!--
> + Since we never want scrollbars, we always use the maxResults value.
> + -->
> + <property name="maxRows"
> + onget="return this.popup.maxResults || Services.prefs.getIntPref(`browser.urlbar.maxRichResults`);"/>
Yes, it was just an overzealous check, but actually I should have checked .popup. It likely doesn't matter so I just removed the fallback.
Comment hidden (mozreview-request) |
Comment 21•7 years ago
|
||
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/07052313bf75
Scrollbar flickers in the Awesomebar results list. r=Paolo
Comment 22•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Iteration: --- → 56.4 - Aug 1
Comment 23•7 years ago
|
||
Verified fix: 56.0a1 20170801100311 - Ubuntu 16.04, Windows 10 x64, Mac OSX 10.12.
You need to log in
before you can comment on or make changes to this bug.
Description
•