Bug 82593 - Assertion fails when a focus navigation traverse a shadow host whose shadow root does not have any children.
Summary: Assertion fails when a focus navigation traverse a shadow host whose shadow r...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 78585
  Show dependency treegraph
 
Reported: 2012-03-29 03:40 PDT by Hayato Ito
Modified: 2012-04-04 19:07 PDT (History)
6 users (show)

See Also:


Attachments
crash case (2.38 KB, patch)
2012-03-29 03:54 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
update a test (3.98 KB, patch)
2012-04-03 23:16 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2012-03-29 03:40:25 PDT
I'll upload a test case.
This should not happen on release build.This happens only on debug build.

Let me explain the detail later. We should revise current traversal APIs so that they can handle the case when ShadowRoot is passed directly as a parameter.
Comment 1 Hayato Ito 2012-03-29 03:54:37 PDT
Created attachment 134542 [details]
crash case
Comment 2 Hayato Ito 2012-03-29 04:19:23 PDT
Let me summarize what I am thinking now:

  - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption:

  node.firstChild.parent == node 

  (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness)

If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken:

     shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot

But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController.
So I think we should change the behaviors as follows

   - If crossing upper boundary is allowed:
     - APIs must not take ShadowRoot as a parameter.
     - APIs won't return ShadowRoot as a return value.

   - If crossing upper boundary is disallowed:
     - APIs can take ShadowRoot as a parameter.
     -Some APIs (parentNode) may return youngest ShadowRoot as a return value.

I think this can solve the issue.
I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs.
Comment 3 Dimitri Glazkov (Google) 2012-03-29 09:15:35 PDT
(In reply to comment #2)
> Let me summarize what I am thinking now:
> 
>   - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption:
> 
>   node.firstChild.parent == node 
> 
>   (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness)
> 
> If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken:
> 
>      shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot
> 
> But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController.
> So I think we should change the behaviors as follows
> 
>    - If crossing upper boundary is allowed:
>      - APIs must not take ShadowRoot as a parameter.
>      - APIs won't return ShadowRoot as a return value.
> 
>    - If crossing upper boundary is disallowed:
>      - APIs can take ShadowRoot as a parameter.
>      -Some APIs (parentNode) may return youngest ShadowRoot as a return value.

I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT?

> I think this can solve the issue.
> I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs.
Comment 4 Hayato Ito 2012-03-30 00:26:06 PDT
(In reply to comment #3)
> I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT?

That's the exactly same approach I had been taken on Walker. I am glad to hear that you have the same opinion by coincidence :).

https://bugs.webkit.org/show_bug.cgi?id=82009 will be ready for a review in a few hours.
Comment 5 Hayato Ito 2012-04-03 23:16:35 PDT
Created attachment 135518 [details]
update a test
Comment 6 WebKit Review Bot 2012-04-04 19:07:44 PDT
Comment on attachment 135518 [details]
update a test

Clearing flags on attachment: 135518

Committed r113275: <http://trac.webkit.org/changeset/113275>
Comment 7 WebKit Review Bot 2012-04-04 19:07:48 PDT
All reviewed patches have been landed.  Closing bug.