Closed Bug 1803873 Opened 1 year ago Closed 1 year ago

Support row buttons in all row types

Categories

(Firefox :: Address Bar, task, P1)

task

Tracking

()

RESOLVED FIXED
109 Branch
Tracking Status
firefox109 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

I'll include a summary in my revision message.

This makes a couple of large changes:

(1) "Generic" buttons (the ones added by UrlbarView.#addRowButton()) are now
supported in all row types. The help button that's currently included in some
types of rows when result.payload.helpUrl is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
result.payload.isBlockable is defined. It will get a labeled button if
result.payload.buttons is defined and non-empty. A button can include a url
property that is then added as an attribute on the button's element, and
UrlbarInput.pickResult() will use this attribute to load the URL when the
button is picked.

(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.

This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
helpUrl and isBlockable are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.

It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to .urlbarView-row, separate from
.urlbarView-row-inner, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.

As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.

Finally, this makes a few other changes:

  • It includes some of the more minor improvements to selection that I made in
    D163630.

  • It removes the help URL code from quick actions since it was decided not to
    show a help button. Currently, the button is hidden in CSS, but now that a
    generic help button is added for dynamic result rows when
    result.payload.helpUrl is defined, helpUrl needs to be removed from the
    payload to prevent a button from being added.

  • I removed the special tip wrapping behavior, where the tip button and help
    button would wrap below the tip's text. Instead, now the text wraps inside
    row-inner and the buttons always remain on the same horizontal as the text. I
    don't think it's worth the extra complication.

Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/263fffe843be
Support row buttons in all row types and make changes to tip rows. r=dao

Backed out for causing mochitest failures on browser_test_focus_urlbar.js

Flags: needinfo?(adw)
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0eac08ef8bc
Support row buttons in all row types and make changes to tip rows. r=dao
Pushed by ctuns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb81f9a1f97e
Support row buttons in all row types and make changes to tip rows. r=dao

Sorry for the backout, that failure turned green, it got relanded: https://hg.mozilla.org/integration/autoland/rev/bb81f9a1f97eff9e4058df8cae47bfcb9270ba82

Flags: needinfo?(adw)
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
Regressions: 1814347
Regressions: 1850334
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: