Closed Bug 1383749 Opened 3 years ago Closed 3 years ago

One-off buttons should be cached the first time

Categories

(Firefox :: Search, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 56
Iteration:
56.4 - Aug 1
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- disabled
firefox56 --- fixed

People

(Reporter: florian, Assigned: florian)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [photon-performance])

Attachments

(1 file)

In bug 1312999 I added code to cache the one-off buttons and rebuild them only if the textbox size changed.

Unfortunately, the current caller code sets the popup before the textbox. Setting popup causes a _rebuild call, which can't cache the textbox size as the textbox setter hasn't been called yet. This causes the reflows in bug 1357054 to happen both the first and second time the awesomebar or searchbar panel is opened, instead of only the first time.
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance]
Attached patch PatchSplinter Review
This wasn't a problem before bug 1369705 as both the popup and textbox setters were called before we had any panel open.
Attachment #8889459 - Flags: review?(mak77)
Keywords: regression
Comment on attachment 8889459 [details] [diff] [review]
Patch

Review of attachment 8889459 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/urlbarBindings.xml
@@ +1684,5 @@
>            this._oneOffSearchesEnabled = enable;
>            if (enable) {
>              this.oneOffSearchButtons.telemetryOrigin = "urlbar";
>              this.oneOffSearchButtons.style.display = "-moz-box";
> +            // The popup setter will cause a _rebuild call, which uses .textbox

Maybe I'd rephrase both comments as
// Set .textbox first, since the popup setter will cause
// a _rebuild call that uses it.
otherwise it's a bit unclear what the comment tries to underline.
Attachment #8889459 - Flags: review?(mak77) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91a2c24475fa
One-off buttons should be cached the first time, r=mak.
Iteration: --- → 56.4 - Aug 1
https://hg.mozilla.org/mozilla-central/rev/91a2c24475fa
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Flags: qe-verify? → qe-verify-
You need to log in before you can comment on or make changes to this bug.