Considering simply arrowscrollbox bindings

NEW
Unassigned

Status

()

P5
normal
3 months ago
2 months ago

People

(Reporter: timdream, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(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.
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

3 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.

You need to log in before you can comment on or make changes to this bug.