Closed Bug 1512043 Opened 6 years ago Closed 5 years ago

When nesting slotted elements input focus change via TAB key doesn't work

Categories

(Core :: DOM: Core & HTML, defect, P2)

Product:
Core
Component:
DOM: Core & HTML
Version:
65 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox66 --- fixed

People

(Reporter: chaephahw0kai2kaugh4, Assigned: edgar)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-66b-p2])

Attachments

(2 files)

reporter's testcase
657 bytes, text/html
Details
Bug 1512043 - Ensure traverse all nodes owned by the top level shadow host;
47 bytes, text/x-phabricator-request
Details | Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

The following code creates two inputs within some nested <div>#shadow-dom(<slot></slot>)</div> elements.

<!DOCTYPE html>
<script>
document.addEventListener('DOMContentLoaded', ev => {
        function shadowEl() {
                let ct = document.createElement('div');
                let s = ct.attachShadow({mode: 'open'});
                let sl = document.createElement('slot');
                s.append(sl);
                return [ct, s, sl];
        }

        let [ct1] = shadowEl();
        document.body.append(ct1);
        let [ct2] = shadowEl();
        let [ct3] = shadowEl();
        ct1.append(ct2, ct3);

        ct2.append(document.createElement('input'));
        ct3.append(document.createElement('input'));
});
</script>



Actual results:

When I press TAB k key inside the first input, focus leaves the page. Shift-TAB doesn't work either to move the focus back.


Expected results:

The focus should move to the next input. On shift-tab the focus should move back.
Component: Untriaged → DOM
Product: Firefox → Core
Attached file reporter's testcase — 

    
    

    
  
This is directly related to shadow dom and should block this issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1496769.

    
  
Blocks: shadowdom
No longer blocks: stop-editor-using-focus/blur-events

    
    

    
  
Ah, wrong link. Sorry. I realized I can set up the dependency myself, so I did.
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
It looks like nsFocusManager::GetNextTabbableContentInAncestorScopes doesn't handle nesting slotted elements well.
Assignee: nobody → echen
Priority: -- → P2

    
    

    
  
(In reply to Edgar Chen [:edgar] from comment #4)
> It looks like nsFocusManager::GetNextTabbableContentInAncestorScopes doesn't
> handle nesting slotted elements well.

GetNextTabbableContentInAncestorScopes stops traversing when we reach an element that is not in shadow tree [1], however the element could be a slotted element, we have to keep looking up from it's `owner` until we reach the top level shadow host.

[1] https://searchfox.org/mozilla-central/rev/dc3585e58b427c3a8a7c01712fe54ebe118a3ce2/dom/base/nsFocusManager.cpp#3149-3156

    
    

    
  
For what it's worth, I've applied the patch to FF 64.0, and tested it with a more complex app that is using web components (where I initially discovered the bug), and it now works as expected after quick testing. Thank you!

    
    

    
  
It is indeed better with FF 64.0 (Thanks for the adjustments!) but still there are some cases which are not covered.

Here: http://embed.plnkr.co/npW8yRXhLGkCV3xmIS1N/
- "content is focusable" focused (red background).
- There are further elements inside the "container-element" which should be focused by pressing tab.
- Press tab key.
--> The activeElement is not the next focusable element inside the "container-element".

Although it is working as expected if the content inside the inner shadowRoot is not focusable but only the element has a tabIndex=0.

    
    

    
  
(In reply to chaephahw0kai2kaugh4 from comment #9)
> For what it's worth, I've applied the patch to FF 64.0, and tested it with a
> more complex app that is using web components (where I initially discovered
> the bug), and it now works as expected after quick testing. Thank you!

Thanks for testing with a more complex case!

    
    

    
  
(In reply to kenyer from comment #10)
> It is indeed better with FF 64.0 (Thanks for the adjustments!) but still
> there are some cases which are not covered.
> 
> Here: http://embed.plnkr.co/npW8yRXhLGkCV3xmIS1N/
> - "content is focusable" focused (red background).
> - There are further elements inside the "container-element" which should be
> focused by pressing tab.
> - Press tab key.
> --> The activeElement is not the next focusable element inside the
> "container-element".
> 
> Although it is working as expected if the content inside the inner
> shadowRoot is not focusable but only the element has a tabIndex=0.

Thanks for reporting this (this seems the same test cases in bug 1510643).
It should work as expected after applying this patch.

    
    

    
  
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/166eac2f9e58
Ensure traverse all nodes owned by the top level shadow host; r=smaug

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/166eac2f9e58
Status: NEW → RESOLVED
Closed: 5 years ago

          status-firefox66:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

    
    

    
  
Thank you!

    
  
Depends on: 1514804
Whiteboard: [qa-66b-p2]
Component: DOM → DOM: Core & HTML

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: