Display one-off search buttons correctly in the UrlbarView panel

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P2
normal
RESOLVED FIXED
10 months ago
5 months ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

https://hg.mozilla.org/mozilla-central/annotate/tip/browser/base/content/browser.xul#l227

There are two options here. We can re-implement this as HTML inside urlbarView-body-inner, or we can just copy the XUL implementation as a sibling to urlbarView-body-outer.
Priority: -- → P2
Summary: Port on-off buttons to the new UrlbarView panel → Port one-off buttons to the new UrlbarView panel
See Also: → 1493536
(In reply to Dão Gottwald [::dao] from comment #0)
> https://hg.mozilla.org/mozilla-central/annotate/tip/browser/base/content/
> browser.xul#l227
> 
> There are two options here. We can re-implement this as HTML inside
> urlbarView-body-inner, or we can just copy the XUL implementation as a
> sibling to urlbarView-body-outer.

Which option are you leaning towards, and what's your timeline here? And would porting the existing search-one-offs to a Custom Element (bug 1493536) somehow interact with the decision? I'm guessing we'll want to ultimately share the same impl between the search popup and URL bar (either a brand new HTML widget, ported XUL CE, or ported XUL CE modified to use HTML).
Flags: needinfo?(dao+bmo)
I'm quite a strong defender of moving all the things to HTML here, because this is important for the extensibility of the URLBarView component; future A/B test experiment _need_ to be able to swap out the one-off buttons using the same mechanism. If we force experiment authors to _again_ be aware of legacy technology - XUL - and mix it with HTML for the results, we're missing the point entirely.

If there's some kind of impediment that blocks an HTML implementation for QuantumBar, let's try to discuss and resolve it early.
As mentioned in bug 1491247, I wasn't suggesting that we keep XUL but that using what we have might be a reasonable first step.

(In reply to Brian Grinstead [:bgrins] from comment #1)
> (In reply to Dão Gottwald [::dao] from comment #0)
> > https://hg.mozilla.org/mozilla-central/annotate/tip/browser/base/content/
> > browser.xul#l227
> > 
> > There are two options here. We can re-implement this as HTML inside
> > urlbarView-body-inner, or we can just copy the XUL implementation as a
> > sibling to urlbarView-body-outer.
> 
> Which option are you leaning towards, and what's your timeline here? And
> would porting the existing search-one-offs to a Custom Element (bug 1493536)
> somehow interact with the decision?

A custom element is likely overkill here. We just need a class that takes a container and does stuff with it, and then we can use that in two places.

I have no timeline but this is needed for reaching parity with the old urlbar, so we'll look into this soon.
Flags: needinfo?(dao+bmo)
Depends on: 1502455
Summary: Port one-off buttons to the new UrlbarView panel → Port one-off search buttons to the new UrlbarView panel
Summary: Port one-off search buttons to the new UrlbarView panel → Make one-off search buttons work with the new UrlbarView panel
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Depends on: 1525269
Attachment #9041451 - Attachment description: Bug 1491248 - Make one-off search buttons work with the new UrlbarView panel. r=Standard8 → Bug 1491248 - Display one-off search buttons correctly in the UrlbarView panel. r=Standard8
Summary: Make one-off search buttons work with the new UrlbarView panel → Display one-off search buttons correctly in the UrlbarView panel
Blocks: 1525269
No longer depends on: 1525269
Blocks: 1525318
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b74ecf5173c2
Display one-off search buttons correctly in the UrlbarView panel. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Depends on: 1528408
Depends on: 1527947
Depends on: 1527934
Depends on: 1528430
You need to log in before you can comment on or make changes to this bug.