migrate wizard binding to CE

NEW
Assigned to

Status

()

P3
normal
4 months ago
12 days ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

4 months ago
Priority: -- → P3
(Assignee)

Comment 1

4 months ago
the wizzard binding changes insertion points for wizardpage elements and all other elements (stringbundle, script). Should I proceed with @slot mechanism and put @slot="wizardpage" attribute on each wizardpage element? or is there nicer approach?
(In reply to alexander :surkov (:asurkov) from comment #1)
> the wizzard binding changes insertion points for wizardpage elements and all
> other elements (stringbundle, script). Should I proceed with @slot mechanism
> and put @slot="wizardpage" attribute on each wizardpage element? or is there
> nicer approach?

It would be nice if there was a way to have the SD be in charge of what gets slotted in, instead of the page having to request it, but that's not part of the spec. I'm not sure why it was specced to flip it so that decision is made by the calling code instead of the shadow host. If we wanted to simulate this, I could imagine writing some code in which the CE sets the [slot] attribute automatically on all child wizardpage's in connectedCallback, and then sets up a MutationObserver to detect when new ones are added and sets the attribute on them as well. From what I hear the MO doesn't have too much overhead, so that could be viable (esp if we aren't watching subtree and just direct children on the CE).

Setting the slot attribute in the calling markup seems OK to me as well.
Or more simply, we can make the wizardpage Custom Element set the correct slot attribute in its constructor.
Depends on: 1495622
See Also: → bug 1495946
(Assignee)

Comment 4

4 months ago
(In reply to Brian Grinstead [:bgrins] from comment #3)
> Or more simply, we can make the wizardpage Custom Element set the correct
> slot attribute in its constructor.

yeah, that one should work (I hope it can't cause some kind of re-insertions and thus lead to intermittents).
(Assignee)

Comment 5

3 months ago
Ran into a problem of nsXULCommandDispatcher::AdvanceFocusIntoSubtree, which hangs when called for a slotted wizardpage. It appears it falls into infinite recursion in nsFocusManager::GetNextTabIndex [1].

[1] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#3991

Olli, do you have ideas how to approach it? Is anyone familiar with this code?
Flags: needinfo?(bugs)
(Assignee)

Comment 6

3 months ago
Created attachment 9017783 [details] [diff] [review]
unified patch
Assignee: nobody → surkov.alexander
See Also: → bug 1496242
testcase please for the hang? But won't be able to look at it before end of this month.
Flags: needinfo?(bugs)
(Assignee)

Comment 8

3 months ago
(In reply to Olli Pettay [:smaug] from comment #7)
> testcase please for the hang?

No tests but steps to reproduce with the patch applied: Bookmarks -> Show all bookmarks -> Import data from another browser -> Continue

> But won't be able to look at it before end of
> this month.

Thank you. It feels a sort of stopper for slotting into shadow DOM though, it doesn't feel specific for wizard CE. If you can think of anyone who could take a look before you, please cc :)

Created attachment 9035849 [details] [diff] [review]
alexander's patch rebased

I rebased the patch and tried comment 8. The browser still hangs when I clicked [Continue].

(Also, the style is missing because document sheet does not apply to Shadow DOM content)

Attachment #9017783 - Attachment is obsolete: true
Attachment #9035849 - Attachment description: alexande's patch rebased → alexander's patch rebased
You need to log in before you can comment on or make changes to this bug.