Changeset 235191 in webkit
- Timestamp:
- Aug 22, 2018 12:59:24 PM (6 years ago)
- Location:
- trunk
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r235181 r235191 1 2018-08-21 Ryosuke Niwa <rniwa@webkit.org> 2 3 Focus navigation order in slot fallback contents is wrong 4 https://bugs.webkit.org/show_bug.cgi?id=178001 5 <rdar://problem/42842997> 6 7 Reviewed by Antti Koivisto. 8 9 Updated the sequential focus navigation test for shadow DOM and its expectation. 10 11 New test passes in Firefox & Chrome other than the fact both browsers fail to focus a slot elemennt. 12 13 * fast/shadow-dom/focus-navigation-across-slots-expected.txt: 14 * fast/shadow-dom/focus-navigation-across-slots.html: 15 1 16 2018-08-22 Per Arne Vollan <pvollan@apple.com> 2 17 -
trunk/LayoutTests/fast/shadow-dom/focus-navigation-across-slots-expected.txt
r200964 r235191 15 15 11. Content in slot 2 with tabindex=1 16 16 12. Content in slot 2 with tabindex=0 17 13. Non-focusable slot fallback with tabindex=118 14. Focusable slot element.17 13. Focusable slot element. 18 14. Focusable slot fallback content with tabindex=0 19 19 15. Shadow content with tabindex=2 20 16. Non-focusable slot fallback with tabindex= 021 17. Focusable slot fallback contentwith tabindex=022 16. Non-focusable slot fallback with tabindex= 020 16. Non-focusable slot fallback with tabindex=1 21 17. Non-focusable slot fallback with tabindex=0 22 16. Non-focusable slot fallback with tabindex=1 23 23 15. Shadow content with tabindex=2 24 14. Focusable slot element.25 13. Non-focusable slot fallback with tabindex=124 14. Focusable slot fallback content with tabindex=0 25 13. Focusable slot element. 26 26 12. Content in slot 2 with tabindex=0 27 27 11. Content in slot 2 with tabindex=1 -
trunk/LayoutTests/fast/shadow-dom/focus-navigation-across-slots.html
r200964 r235191 103 103 <slot name="slot1" onfocus="log(this)"> 104 104 Non-focusable slot should not be focused. 105 <div tabindex="0">1 6. Non-focusable slot fallback with tabindex=0</div>106 <div tabindex="1">1 3. Non-focusable slot fallback with tabindex=1</div>105 <div tabindex="0">17. Non-focusable slot fallback with tabindex=0</div> 106 <div tabindex="1">16. Non-focusable slot fallback with tabindex=1</div> 107 107 </slot> 108 108 <div tabindex="2" onfocus="log(this)">15. Shadow content with tabindex=2</div> 109 109 <slot name="slot2" tabindex="1" style="display:block;" onfocus="log(this)"> 110 1 4. Focusable slot element.111 <div tabindex="0">1 7. Focusable slot fallback content with tabindex=0</div>110 13. Focusable slot element. 111 <div tabindex="0">14. Focusable slot fallback content with tabindex=0</div> 112 112 </slot>`; 113 113 -
trunk/Source/WebCore/ChangeLog
r235190 r235191 1 2018-08-21 Ryosuke Niwa <rniwa@webkit.org> 2 3 Focus navigation order in slot fallback contents is wrong 4 https://bugs.webkit.org/show_bug.cgi?id=178001 5 <rdar://problem/42842997> 6 7 Reviewed by Antti Koivisto. 8 9 The bug here is that when a slot uses its fallback content, the fallback content's focus order doesn't get 10 grouped by that of the slot. Consider the following DOM tree: 11 12 - ShadowRoot 13 - div tabindex = 2 14 - slot tabindex = 1 15 - span tabindex = 3 16 17 In this example, the sequential focus navigation should be slot, span, then div. Even though span has tabindex 18 order of 3, which is lower than that of div, the fallback content of the slot should be grouped together 19 before the focus moves out of the slot content. 20 21 In WebKit, this concept of grouping elements for the sequential focus navigation ordering is implemeneted 22 as FocusNavigationScope. Both ShadowRoot and HTMLSlotElement are treated as a focus scope owner but we had 23 a bug that a slot element which uses its fallback content was not treated as a focus scope owner. 24 25 This patch addresses the bug by treating a slot wich uses its fallback content as a focus scope owner. 26 27 Test: fast/shadow-dom/focus-navigation-across-slots.html 28 29 * page/FocusController.cpp: 30 (WebCore::isFocusScopeOwner): Treat a slot elment hs a focus scope owner regardless of whether it has assigned 31 nodes or not. 32 (WebCore::FocusNavigationScope::SlotKind): Added. 33 (WebCore::FocusNavigationScope::m_slotKind): Added. 34 (WebCore::FocusNavigationScope::parentInScope const): Return null if `node` is a child of the slot element for 35 which this FocusNavigationScope is created (i.e. `node` is slot's fallback content). 36 (WebCore::FocusNavigationScope::firstNodeInScope const): Return the first child node when this 37 FocusNavigationScope is for a slot element using its fallback content. 38 (WebCore::FocusNavigationScope::lastNodeInScope const): Ditto for the last child. 39 (WebCore::FocusNavigationScope::FocusNavigationScope): 40 (WebCore::FocusNavigationScope::scopeOf): The scope of a child of a slot element which uses its fallback content 41 is its slot element (i.e. the current node is a fallback content). We can't simply check the current node is 42 a slot element which uses a fallback content since the scope of a slot element is the parent scope. e.g. its 43 tree scope like ShadowRoot or Document inside which this slot element appears. 44 (WebCore::FocusNavigationScope::scopeOwnedByScopeOwner): Create the appropriate FocusNavigationScope based on 45 whether the slot element has assigned or it uses its fallback content. 46 1 47 2018-08-22 David Kilzer <ddkilzer@apple.com> 2 48 -
trunk/Source/WebCore/page/FocusController.cpp
r234995 r235191 76 76 if (element.shadowRoot() && !hasCustomFocusLogic(element)) 77 77 return true; 78 if (is<HTMLSlotElement>(element) && downcast<HTMLSlotElement>(element).assignedNodes()) {78 if (is<HTMLSlotElement>(element)) { 79 79 ShadowRoot* root = element.containingShadowRoot(); 80 80 if (root && root->host() && !hasCustomFocusLogic(*root->host())) … … 98 98 99 99 private: 100 enum class SlotKind : uint8_t { Assigned, Fallback }; 101 100 102 Node* firstChildInScope(const Node&) const; 101 103 … … 106 108 107 109 explicit FocusNavigationScope(TreeScope&); 108 109 explicit FocusNavigationScope(HTMLSlotElement&); 110 explicit FocusNavigationScope(HTMLSlotElement&, SlotKind); 110 111 111 112 TreeScope* m_rootTreeScope { nullptr }; 112 113 HTMLSlotElement* m_slotElement { nullptr }; 114 SlotKind m_slotKind { SlotKind::Assigned }; 113 115 }; 114 116 … … 133 135 return nullptr; 134 136 135 if (UNLIKELY(m_slotElement && m_slotElement == node.assignedSlot())) 136 return nullptr; 137 if (UNLIKELY(m_slotElement)) { 138 if (m_slotKind == SlotKind::Assigned) { 139 if (m_slotElement == node.assignedSlot()) 140 return nullptr; 141 } else { 142 ASSERT(m_slotKind == SlotKind::Fallback); 143 auto* parentNode = node.parentNode(); 144 if (parentNode == m_slotElement) 145 return nullptr; 146 } 147 } 137 148 138 149 return node.parentNode(); … … 167 178 if (UNLIKELY(m_slotElement)) { 168 179 auto* assigneNodes = m_slotElement->assignedNodes(); 169 ASSERT(assigneNodes); 170 return assigneNodes->first(); 180 if (m_slotKind == SlotKind::Assigned) { 181 ASSERT(assigneNodes); 182 return assigneNodes->first(); 183 } 184 ASSERT(m_slotKind == SlotKind::Fallback); 185 return m_slotElement->firstChild(); 171 186 } 172 187 ASSERT(m_rootTreeScope); … … 178 193 if (UNLIKELY(m_slotElement)) { 179 194 auto* assigneNodes = m_slotElement->assignedNodes(); 180 ASSERT(assigneNodes); 181 return assigneNodes->last(); 195 if (m_slotKind == SlotKind::Assigned) { 196 ASSERT(assigneNodes); 197 return assigneNodes->last(); 198 } 199 ASSERT(m_slotKind == SlotKind::Fallback); 200 return m_slotElement->lastChild(); 182 201 } 183 202 ASSERT(m_rootTreeScope); … … 214 233 } 215 234 216 FocusNavigationScope::FocusNavigationScope(HTMLSlotElement& slotElement )235 FocusNavigationScope::FocusNavigationScope(HTMLSlotElement& slotElement, SlotKind slotKind) 217 236 : m_slotElement(&slotElement) 237 , m_slotKind(slotKind) 218 238 { 219 239 } … … 236 256 { 237 257 ASSERT(startingNode.isInTreeScope()); 238 Node* root = nullptr; 239 for (Node* currentNode = &startingNode; currentNode; currentNode = currentNode->parentNode()) { 258 RefPtr<Node> root; 259 RefPtr<Node> parentNode; 260 for (RefPtr<Node> currentNode = &startingNode; currentNode; currentNode = parentNode) { 240 261 root = currentNode; 241 262 if (HTMLSlotElement* slot = currentNode->assignedSlot()) { 242 263 if (isFocusScopeOwner(*slot)) 243 return FocusNavigationScope(*slot );264 return FocusNavigationScope(*slot, SlotKind::Assigned); 244 265 } 245 266 if (is<ShadowRoot>(currentNode)) 246 267 return FocusNavigationScope(downcast<ShadowRoot>(*currentNode)); 268 parentNode = currentNode->parentNode(); 269 // The scope of a fallback content of a HTMLSlotElement is the slot element 270 // but the scope of a HTMLSlotElement is its parent scope. 271 if (parentNode && is<HTMLSlotElement>(parentNode) && !downcast<HTMLSlotElement>(*parentNode).assignedNodes()) 272 return FocusNavigationScope(downcast<HTMLSlotElement>(*parentNode), SlotKind::Fallback); 247 273 } 248 274 ASSERT(root); … … 253 279 { 254 280 ASSERT(element.shadowRoot() || is<HTMLSlotElement>(element)); 255 if (is<HTMLSlotElement>(element)) 256 return FocusNavigationScope(downcast<HTMLSlotElement>(element)); 281 if (is<HTMLSlotElement>(element)) { 282 auto& slot = downcast<HTMLSlotElement>(element); 283 return FocusNavigationScope(slot, slot.assignedNodes() ? SlotKind::Assigned : SlotKind::Fallback); 284 } 257 285 return FocusNavigationScope(*element.shadowRoot()); 258 286 }
Note: See TracChangeset
for help on using the changeset viewer.