migrate wizard binding to CE
Categories
(Core :: XUL, task, P3)
Tracking
()
People
(Reporter: surkov, Assigned: surkov)
References
(Depends on 1 open bug)
Details
Attachments
(4 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Comment 3•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Comment 5•6 years ago
|
||
Assignee | ||
Comment 6•6 years ago
|
||
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
Comment 9•6 years ago
|
||
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)
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 10•6 years ago
|
||
Assignee | ||
Comment 11•6 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo; no longer work for MoCo) from comment #9)
Created attachment 9035849 [details] [diff] [review]
alexander's patch rebasedI rebased the patch and tried comment 8. The browser still hangs when I clicked [Continue].
rebased the patch, hangs present
(Also, the style is missing because document sheet does not apply to Shadow DOM content)
I linked widgets styles into markup, now it seems ok
Comment 13•6 years ago
|
||
Edgar fixed some focus issues inside Shadow DOM not long ago. Is this issue still present? I guess it is per comment 11, and relevant str is still comment 8.
I was going to ni? him since he had more chances to remember this code, but he's on leave. I'll try to take a look at some point of the week.
Assignee | ||
Comment 14•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)
Edgar fixed some focus issues inside Shadow DOM not long ago. Is this issue still present? I guess it is per comment 11, and relevant str is still comment 8.
yeah, still an issue
I was going to ni? him since he had more chances to remember this code, but he's on leave. I'll try to take a look at some point of the week.
thank you!
Comment 15•6 years ago
|
||
I could import bookmarks from other browsers without issues, what am I doing wrong? :)
Assignee | ||
Comment 16•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
I could import bookmarks from other browsers without issues, what am I doing wrong? :)
hm, are you sure you have the patch applied: clicking 'Continue' in Import Wizard Dialog, hangs the browser for me. If I click 'Create Profile' button in Profile Dialog (./mach run -P), then the browser crashes. I can see that on OS X on today's locally built central.
Comment 17•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #16)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
I could import bookmarks from other browsers without issues, what am I doing wrong? :)
hm, are you sure you have the patch applied: clicking 'Continue' in Import Wizard Dialog, hangs the browser for me. If I click 'Create Profile' button in Profile Dialog (./mach run -P), then the browser crashes. I can see that on OS X on today's locally built central.
Pretty sure, I can also repro that crash. Focus handling on OSX is quite a bit different from Linux though.
Assignee | ||
Comment 18•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)
(In reply to alexander :surkov (:asurkov) from comment #16)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
I could import bookmarks from other browsers without issues, what am I doing wrong? :)
hm, are you sure you have the patch applied: clicking 'Continue' in Import Wizard Dialog, hangs the browser for me. If I click 'Create Profile' button in Profile Dialog (./mach run -P), then the browser crashes. I can see that on OS X on today's locally built central.
Pretty sure, I can also repro that crash. Focus handling on OSX is quite a bit different from Linux though.
I hope you can help with the crash then, and then let's cross the fingers the crash fixes the hang :)
Comment 20•6 years ago
|
||
Well, the crash happens because of the <link rel="stylesheet">
that you're
adding. We have code to lazily query the visited state of that link, for
which we need the history service. But obviously there's no profile yet when
that happens.
Other browsers don't seem to support styling <link>
elements with
:visited
, fwiw, but we do. The second <link>
element should be purple.
Seems like other browsers don't ever match :link
/ :any-link
on <link>
elements.
<style>
body { margin: 0 }
a, head, link { display: block }
a, link { width: 100px; height: 100px; background-color: red }
:any-link { background-color: blue }
a:visited, link:visited { background-color: purple }
</style>
<link href="https://google.es">
<link href="">
<a href="https://mozilla.org"></a>
<a href=""></a>
Per spec, I think we're right:
https://drafts.csswg.org/selectors-4/#location
Or at least, don't see anything that would make <link>
special there.
But I don't think there's any use-case for this, and I think we could just make
<link>
not respect the link state.
Assignee | ||
Comment 21•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)
Well, the crash happens because of the
<link rel="stylesheet">
that you're
adding. We have code to lazily query the visited state of that link, for
which we need the history service. But obviously there's no profile yet when
that happens.
widgets.css is linked to a shadow DOM of xul:tree custom element already, https://searchfox.org/mozilla-central/source/toolkit/content/widgets/tree.js#515. If presence of :visited style is a problem, then it should crash as well, no?
Per spec, I think we're right:
https://drafts.csswg.org/selectors-4/#location
Or at least, don't see anything that would make
<link>
special there.But I don't think there's any use-case for this, and I think we could just make
<link>
not respect the link state.
Is it something you could fix?
Comment 22•6 years ago
|
||
https://github.com/w3c/csswg-drafts/issues/3817 is the spec issue, will write a work-around applicable only for XUL documents for now.
Comment 23•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #21)
widgets.css is linked to a shadow DOM of xul:tree custom element already, https://searchfox.org/mozilla-central/source/toolkit/content/widgets/tree.js#515. If presence of :visited style is a problem, then it should crash as well, no?
Well, it's only a problem if there's no profile at the time we query the visited state. Which looks like it can only happen in a few selected places.
Assignee | ||
Comment 24•6 years ago
|
||
I cannot reproduce a hang when I click 'Continue' button in Profile Manager, however it hangs when I press 'Back' button afterwards. It falls into infinite recursion in nsFocusManager::GetNextTabbableContentInAncestorScopes, where startContent and its owner is a wizard element, see https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#3202.
Emilio, any ideas on this?
Assignee | ||
Comment 25•6 years ago
|
||
Another problem is in test_update_wizard.py test [1] which fails to grab wizard-buttons, a shadow DOM element of <wizard> element, in wizard.py [2].
I expected that changing it to
self.element.find_element(By.CSS_SELECTOR, '.wizard-buttons')
will work out per [3], but apparently |shadowRoot| is never set [4].
Not sure, who could help with it. Maybe :ato has ideas?
[1] ./mach test testing/firefox-ui/tests/puppeteer/test_update_wizard.py TestUpdateWizard.test_elements
[2] https://searchfox.org/mozilla-central/source/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/update_wizard/wizard.py#53
[3] https://searchfox.org/mozilla-central/source/testing/marionette/element.js#279
[4] https://searchfox.org/mozilla-central/source/testing/marionette/driver.js#2009
Comment 26•6 years ago
|
||
I don't expect this is it - but is it possible the element hasn't been upgraded yet due to some weirdness with it being used in a XBL binding or something? Like if you force container.ownerDocument.defaultView.customElements.upgrade(container)
before https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516affe4d28cb7fa220a/testing/marionette/element.js#352 do you see the same thing? And can you confirm that shadowRoot is null on that line for the failing case?
Assignee | ||
Comment 27•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #26)
I don't expect this is it - but is it possible the element hasn't been upgraded yet due to some weirdness with it being used in a XBL binding or something? Like if you force
container.ownerDocument.defaultView.customElements.upgrade(container)
before https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516affe4d28cb7fa220a/testing/marionette/element.js#352 do you see the same thing? And can you confirm that shadowRoot is null on that line for the failing case?
I prototyped find_element extension to get an element from a property, it works. So it shouldn't be an upgrade issue.
Comment 28•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #24)
I cannot reproduce a hang when I click 'Continue' button in Profile Manager, however it hangs when I press 'Back' button afterwards. It falls into infinite recursion in nsFocusManager::GetNextTabbableContentInAncestorScopes, where startContent and its owner is a wizard element, see https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#3202.
Emilio, any ideas on this?
I cannot repro that, neither when creating a profile nor when importing bookmarks from another browser.
But does that mean that FindOwner(wizard) returns the same wizard element? That's bad, since FindOwner always looks at an ancestor.
Comment 29•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #25)
Another problem is in test_update_wizard.py test [1] which fails
to grab wizard-buttons, a shadow DOM element of <wizard> element,
in wizard.py [2].
Marionette doesn’t support Shadow DOM, but the trick bgrins mentioned
might help?
Comment 30•6 years ago
|
||
(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #29)
(In reply to alexander :surkov (:asurkov) from comment #25)
Another problem is in test_update_wizard.py test [1] which fails
to grab wizard-buttons, a shadow DOM element of <wizard> element,
in wizard.py [2].Marionette doesn’t support Shadow DOM
It references shadowRoot at https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516affe4d28cb7fa220a/testing/marionette/element.js#352. What type of object is container
here?
Comment 31•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #30)
(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #29)
Marionette doesn’t support Shadow DOM
It references shadowRoot at
https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516a
ffe4d28cb7fa220a/testing/marionette/element.js#352. What type of
object iscontainer
here?
I thought it was an implementation of one of the very first versions
of Shadow DOM, so I wouldn’t expect it to work. Though there is a
test in
testing/marionette/harness/marionette_harness/tests/unit/test_shadow_dom.py
that seems to indicate that it does. I haven’t checked if what it
does is correct. AutomatedTester may know more.
container
is a {"window": <WindowProxy>, "shadowRoot": <Element>}
.
Assignee | ||
Comment 32•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #28)
(In reply to alexander :surkov (:asurkov) from comment #24)
I cannot reproduce a hang when I click 'Continue' button in Profile Manager, however it hangs when I press 'Back' button afterwards. It falls into infinite recursion in nsFocusManager::GetNextTabbableContentInAncestorScopes, where startContent and its owner is a wizard element, see https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#3202.
Emilio, any ideas on this?
I cannot repro that, neither when creating a profile nor when importing bookmarks from another browser.
But does that mean that FindOwner(wizard) returns the same wizard element? That's bad, since FindOwner always looks at an ancestor.
FindOwner returns a wizard for wizard, because wizard is a root element, and thus it has no nsIContent parent and FindOwner fallsbacks into a root element, which is a wizard element [1].
I'm not sure what is expected behavior of FindOwner, but the hang can be fixed if nsFocusManager::GetNextTabbableContentInAncestorScopes will compare startContent and owner and will break the loop if they match. I'm not sure however if there are any other underlying issues leading to this problem.
[1] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2999
Comment 33•6 years ago
|
||
There's definitely some oddness when the shadow root is the root element. Compare this test-case with and without the onload handler commented out:
<!doctype html>
<div id="host"><a href="#">This is focusable too</a></div>
<script>
let host = document.getElementById("host");
host.attachShadow({ mode: "open" }).innerHTML = `
<a href="#">This is focusable</a>
<slot></slot>
<a href="#">So is this</a>
`;
onload = function () {
document.documentElement.remove();
document.appendChild(host);
}
</script>
Will file a bug for that.
Comment 34•6 years ago
|
||
Can you confirm that the patch in bug 1544826 fixes the issue you're seeing?
Assignee | ||
Comment 35•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #34)
Can you confirm that the patch in bug 1544826 fixes the issue you're seeing?
it doesn't, let me see where it hangs now
Assignee | ||
Comment 36•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #35)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #34)
Can you confirm that the patch in bug 1544826 fixes the issue you're seeing?
it doesn't, let me see where it hangs now
It keeps reentering GetNextTabbableContent for a wizard element as a rootContent/startContent [1] in the while(doc) loop inside FocusManager::DetermineElementToMoveFocus(), in the end of the loop null startContent is assigned to rootContent and the loop keeps rolling all over again.
[1] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2728
[2] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2862
Comment 37•6 years ago
|
||
So I assume that forDocumentNavigation
is false
, right? How's the callstack that gets you there?
Because otherwise I guess you're supposed to hit:
And break out. So that probably can happen when navigating around when no single focusable element exists at all...
The other thing that is supposed to avoid hanging there is:
What are the values of originalStartContent
and startContent
as you iterate?
Assignee | ||
Comment 38•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #37)
So I assume that
forDocumentNavigation
isfalse
, right?
correct
How's the callstack that gets you there?
#6 0x000000011ba5d67f in nsFocusManager::DetermineElementToMoveFocus(nsPIDOMWindowOuter*, nsIContent*, int, bool, nsIContent**) at /Users/heh/mozilla/central/dom/base/nsFocusManager.cpp:2746
#7 0x000000011ba5c724 in nsFocusManager::MoveFocus(mozIDOMWindowProxy*, mozilla::dom::Element*, unsigned int, unsigned int, mozilla::dom::Element**) at /Users/heh/mozilla/central/dom/base/nsFocusManager.cpp:523
#8 0x000000011ceda089 in nsXULCommandDispatcher::AdvanceFocusIntoSubtree(mozilla::dom::Element*) at /Users/heh/mozilla/central/dom/xul/nsXULCommandDispatcher.cpp:213
Because otherwise I guess you're supposed to hit:
And break out. So that probably can happen when navigating around when no single focusable element exists at all...
The other thing that is supposed to avoid hanging there is:
What are the values of
originalStartContent
andstartContent
as you iterate?
originalStartContent is a wizardpage (<wizardpage id='explanation'>), startContent is a wizard itself (<wizard id='createProfileWizard'>).
Assignee | ||
Updated•6 years ago
|
Comment 39•6 years ago
|
||
So originalStartContent
is the slotted content of the wizard, I guess? How do we get to that situation? Is that element somehow focusable? If so, how do we end up not finding it anymore?
Assignee | ||
Comment 40•6 years ago
|
||
Assignee | ||
Comment 41•6 years ago
|
||
crash healed itself, so the only blocking is left is failing marionette tests, but it appears I have a fix for this case.
Assignee | ||
Comment 42•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #41)
crash healed itself, so the only blocking is left is failing marionette tests, but it appears I have a fix for this case.
so the crash didn't heal itself completely, it became an intermittent one - https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=245010505&repo=try&lineNumber=4616. I cannot reproduce it locally, which is sad.
Assignee | ||
Comment 43•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #42)
(In reply to alexander :surkov (:asurkov) from comment #41)
crash healed itself, so the only blocking is left is failing marionette tests, but it appears I have a fix for this case.
so the crash didn't heal itself completely, it became an intermittent one - https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=245010505&repo=try&lineNumber=4616. I cannot reproduce it locally, which is sad.
this crash actually is something new, however it also related with advanceFocusIntoSubtree. I tried to pass a document into advanceFocusIntoSubtree (the crash stack is above), and a wizard page (crash stack is https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=245113033&repo=try&lineNumber=17555).
Emilio, do the stacks say anything to you?
Comment 44•6 years ago
|
||
Not much, that's just where the process was killed. The entry point for that stack is XULCommandDispatcher::AdvanceFocusIntoSubtree called from JS. Do you know what the JS stack looks at that point? (You can see it calling DumpJSStack()
from the debugger).
Assignee | ||
Comment 45•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #44)
Not much, that's just where the process was killed. The entry point for that stack is XULCommandDispatcher::AdvanceFocusIntoSubtree called from JS. Do you know what the JS stack looks at that point? (You can see it calling
DumpJSStack()
from the debugger).
I think this is AdvanceFocusIntoSubtree called by a wizard on document "onload" after timeout (setTimeout(0)). Does it help?
I can't reproduce it locally, is there DumpJSStack() options available on try?
Comment 46•6 years ago
|
||
Comment 47•6 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #45)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #44)
Not much, that's just where the process was killed. The entry point for that stack is XULCommandDispatcher::AdvanceFocusIntoSubtree called from JS. Do you know what the JS stack looks at that point? (You can see it calling
DumpJSStack()
from the debugger).I think this is AdvanceFocusIntoSubtree called by a wizard on document "onload" after timeout (setTimeout(0)). Does it help?
I can't reproduce it locally, is there DumpJSStack() options available on try?
Not much, no, I'd really need to be able to reproduce this with a debugger :(
DumpJSStack() should be available on debug builds, you should be able to call it from AdvanceFocusIntoSubtree or such.
Comment 48•6 years ago
|
||
bugherder |
Updated•6 years ago
|
Updated•6 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 49•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #39)
So
originalStartContent
is the slotted content of the wizard, I guess? How do we get to that situation? Is that element somehow focusable? If so, how do we end up not finding it anymore?
originalStartContent is a slotted content to the wizzard, correct. So originally the loop is entered for the slotted <wizardpage id='explanation'> element, but when we fail to find a focusable content inside it by GetNextTabbableContent [1], then startContent is dropped to root at [2], and it loops endlessly. I wonder if originalContent should be dropped to root as well to make this condition working [3]. At least it fixes the hang.
[1] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2741
[2] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2876
[3] https://searchfox.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2882
Comment 50•5 years ago
|
||
Why doesn't the algorithm visit originalStartContent
again?
Assignee | ||
Comment 51•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #50)
Why doesn't the algorithm visit
originalStartContent
again?
I afraid I didn't I capture your question, this loop keeps calling GetNextTabbableContent for the originalStartContent.
Comment 52•5 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #49)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #39)
So
originalStartContent
is the slotted content of the wizard, I guess? How do we get to that situation? Is that element somehow focusable? If so, how do we end up not finding it anymore?originalStartContent is a slotted content to the wizzard, correct. So originally the loop is entered for the slotted <wizardpage id='explanation'> element, but when we fail to find a focusable content inside it by GetNextTabbableContent [1], then startContent is dropped to root at [2], and it loops endlessly. I wonder if originalContent should be dropped to root as well to make this condition working [3]. At least it fixes the hang.
Honestly it's hard to say without being able to reproduce it locally. It only happens on mac, right? If you're on a mac, can you repro it? I tried on Linux with a mac-like set of a11y prefs and I couldn't get it to repro.
Assignee | ||
Comment 53•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #52)
Honestly it's hard to say without being able to reproduce it locally. It only happens on mac, right? If you're on a mac, can you repro it? I tried on Linux with a mac-like set of a11y prefs and I couldn't get it to repro.
yeah, it happens on mac and I can reproduce it locally: just clicking 'Create Profile' button in profile manager window triggers it.
Assignee | ||
Comment 54•5 years ago
|
||
Comment 55•5 years ago
|
||
Is the issue that there is nothing focusable in the window? (on Mac buttons aren't tab-focusable, IIRC).
But what happen currently, without CE?
Comment 56•5 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #55)
Is the issue that there is nothing focusable in the window? (on Mac buttons aren't tab-focusable, IIRC).
But what happen currently, without CE?
Right, that's my theory, but then I should be able to reproduce this with accessibility.tabfocus=1
on Linux, but I don't.
Oh, but as I'm writing this I realized that all the #ifdef XP_MACOSX
stuff in there does affect the test (I thought not) because the test uses the mouse.
Comment 57•5 years ago
|
||
The following is the log I get with this patch applied:
[(null) 83469: Main Thread]: D/FocusNavigation Focus Navigation Start Content wizardpage
[(null) 83469: Main Thread]: D/FocusNavigation Forward: 1 Tabindex: 1 Ignore: 1 DocNav: 0
[(null) 83469: Main Thread]: D/FocusNavigation GetNextTabbable: wizardpage
[(null) 83469: Main Thread]: D/FocusNavigation tabindex: 1
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable popupgroup:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 0
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable hbox:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 0
<----------------- Here is where the patch above sets 'breakReentry' to true
[(null) 83469: Main Thread]: D/FocusNavigation GetNextTabbable: wizard
[(null) 83469: Main Thread]: D/FocusNavigation tabindex: 1
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable wizard:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 1
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable popupgroup:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 1
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable hbox:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 1
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable popupgroup:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 0
[(null) 83469: Main Thread]: D/FocusNavigation Next Tabbable hbox:
[(null) 83469: Main Thread]: D/FocusNavigation with tabindex: -1 expected: 0
The starting wizardpage is never visited again at least for the logging done here.
Assignee | ||
Comment 58•5 years ago
|
||
Curious, how does the log with no patch applied look like? How do you enable this logging?
Assignee | ||
Comment 59•5 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #55)
Is the issue that there is nothing focusable in the window? (on Mac buttons aren't tab-focusable, IIRC).
it keeps re-entrying for <wizardpage id='explanation'> (see comment #49), which contains no focusables inside [1]. I suppose something in the window should be focusable (maybe not tab focusable though): wizard buttons or at least document itself.
[1] https://searchfox.org/mozilla-central/source/toolkit/profile/content/createProfileWizard.xul#29
Comment 60•5 years ago
|
||
Assignee | ||
Comment 61•5 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #55)
But what happen currently, without CE?
currently it goes through anonymous content of XBL wizard binding twice (!): wizard header, then wizardpages (each of them twice again) and then it finishes by traversing anonymous XBL buttons, then it ends up with "Element to be focused: (none)". Does it look like something is broken (or not optimized at least) that it traverses same things twice?
After the patch, it enters for a first wizardpage (wizardpage@id='explanation'), not for a wizard itself, then it jumps to the wizard, but it seems never traverse its shadow content.
Comment 62•5 years ago
|
||
I'd expect it to traverse with different tabindexes.
Assignee | ||
Comment 63•5 years ago
|
||
I filed bug 1558990 for hanging issue.
Assignee | ||
Comment 64•5 years ago
|
||
(In reply to alexander :surkov (:asurkov) from comment #63)
I filed bug 1558990 for hanging issue.
so, wizard hangs when I advanceFocusIntoSubtree is called on a slotted wizardpage (see bug 1558990 for isolated testcase). The original wizard's version used to call advanceFocusIntoSubtree for a document element, so I reverted the behavior for CE wizard, and it solved a problem. So filed bug 1558990, since it's a separate issue and doesn't longer block this one.
I started a try server build and if it's green, then this wizard thing can be landed.
Updated•5 years ago
|
Comment 65•5 years ago
|
||
Comment 66•5 years ago
|
||
Backed out for marionette mozprocess timed out failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/5263ebe8baf2a3c76b224e547b960c2e68057d88
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=os%2Cx%2C10.14%2Cshippable%2Copt%2Ctest-macosx1014-64-shippable%2Fopt-marionette-e10s%2C%28mn%29&revision=62ac95a65617d4a0bdcf494986d4b9c9626cc7f9&selectedJob=251725222
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=251725321&repo=autoland&lineNumber=55597
Comment 67•5 years ago
|
||
bugherder |
Comment 68•5 years ago
|
||
(In reply to Stefan Hindli [:stefan_hindli] from comment #67)
I'm confused - shouldn't the backout (Comment 66 / https://hg.mozilla.org/integration/autoland/rev/5263ebe8baf2a3c76b224e547b960c2e68057d88) have merged alongside this, and this not have been resolved?
If it didn't I expect the Mn failures from Comment 66 to be on m-c.
Comment 69•5 years ago
|
||
:bgrins we will merge the backout in ~ 2 hours if everything goes well. The plan was to merge the backout also but there were some issues with some failed jobs and decided to use an earlier changeset.
Comment 70•5 years ago
|
||
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/62ac95a65617
Assignee | ||
Comment 71•5 years ago
|
||
(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #70)
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/62ac95a65617
wizard tries to move the focus into a slotted wizardpage, and it hangs, so we need bug 1558990 get resolved before landing this one.
Comment 72•5 years ago
|
||
Comment 73•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Description
•