Summary: | AX: AX hit-testing does not work on WebKit video playback buttons | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nan Wang <n_wang> | ||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | n_wang, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Nan Wang
2016-03-31 14:48:47 PDT
Created attachment 275350 [details]
initial patch
Created attachment 275354 [details]
patch
test failed on DRT
Comment on attachment 275354 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=275354&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2260 > + if (!shadowHost) > + return false; > + > + if (shadowHost->hasTagName(videoTag)) > + return false; > + > + return true; Cleaner way to write this: return shadowHost && shadowHost->hasTagName(videoTag); But also, this needs a comment explaining why this is the correct implementation for this function. What is special about video tags? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2277 > + Node* node = hitTestResult.innerNode(); Should put this into a local variable before the early return just above. Node* node = hitTestResult.innerNode(); if (!node) return nullptr; Comment on attachment 275354 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=275354&action=review >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2260 >> + return true; > > Cleaner way to write this: > > return shadowHost && shadowHost->hasTagName(videoTag); > > But also, this needs a comment explaining why this is the correct implementation for this function. What is special about video tags? We need to allow automation of mouse events on video tags. Committed r198941: <http://trac.webkit.org/changeset/198941> |