Convert arrowscrollbox binding to a custom element

NEW
Unassigned

Status

()

task
P5
normal
6 months ago
12 days ago

People

(Reporter: timdream, Unassigned)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Per what's suggested in the meeting, we should be able to flatten arrowscrollbox and arrowscrollbox-clicktoscroll into one custom element, and move tabbrowser-arrowscrollbox to its (sole?) user.
Priority: -- → P5
Attachment #9033043 - Attachment is obsolete: true
Attachment #9033043 - Attachment is obsolete: false
I've moved https://phabricator.services.mozilla.com/D14925 here because I will need that to be able to inherit from BaseControl.

Other than that, here are the findings:

* The element is not used in XUL documents excepts tests.
* It is used in popup, menu, and tabbrowser bindings.

This is actually a nice use case of Shadow DOM, unfortunately because of bug 1515518 I will need to think of alternatives...
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2)
> * It is used in popup, menu, and tabbrowser bindings.

Sorry, I should be more specific.

The IDs of the bindings are places-popup-arrow, places-popup-base, popup, and tabbrowser-tabs.

Updated

6 months ago
Depends on: 1516266
Attachment #9033043 - Attachment is obsolete: true
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2)
> I've moved https://phabricator.services.mozilla.com/D14925 here because I
> will need that to be able to inherit from BaseControl.

... and I was wrong about this.
On top of bug 1516266. This converts the arrowscrollbox binding and
tabbrowser-arrowscrollbox binding to custom elements.

The debug browser will fail to launch with

Assertion failure: !content->IsActiveChildrenElement(), at /builds/worker/workspace/build/src/layout/base/RestyleManager.cpp:3033

Appearently because we are moving the <children> element in the connected callback.
Also, trying to locate the children like |document.getBindingParent(this).children;| and move them manually into <scrollbox> also failed with the same assertion error.

https://searchfox.org/mozilla-central/rev/7a922172a94cfe24c7a48e0a581577895e1da8c4/toolkit/content/widgets/scrollbox.xml#349
Actually, it failed with

Assertion failure: content->GetFlattenedTreeParentNodeForStyle() (Node not in the flattened tree still has a frame?), at /builds/worker/workspace/build/src/layout/base/PresShell.cpp:3946

Unassigning. This is not actionable for now.

Assignee: timdream → nobody
Status: ASSIGNED → NEW
Attachment #9033363 - Attachment is obsolete: true

Note that the patch may work without hitting assertion as soon as we have removed all <arrowscrollbox> usage inside XBL <content>

https://searchfox.org/mozilla-central/search?q=%3Cxul%3Aarrowscrollbox&case=false&regexp=false&path=

At the time of writing these bindings are

  • tabbrowser-tabs
  • places-popup-base
  • places-popup-arrow
  • popup

Dependencies should be set when bugs for these bindings are filed.

Depends on: 1539665
Summary: Considering simply arrowscrollbox bindings → Consider simplying arrowscrollbox bindings

Updated

2 months ago
Summary: Consider simplying arrowscrollbox bindings → Convert arrowscrollbox binding to a custom element

Updated

2 months ago
Type: enhancement → task

here's updated to trunk patch. It uses shadow DOM which requires to move styling into the widget or wait for bug 1505489. Also, it appears both approaches (shadow DOM and the light DOM original approach) will require adjustments in keyboard handling in popup code (see bug 1555497 for details), so bug 1555497 might be a stopper for this one.

Depends on: shadow-parts
You need to log in before you can comment on or make changes to this bug.