Bug 48817

Summary: Chromium: Propagate a document value changed notification on scroll.
Product: WebKit Reporter: Chris Guillory <ctguil>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, commit-queue, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Send document value changed on scrollbar value changed notification.
none
Proposed patch.
cfleizach: review-, cfleizach: commit-queue-
Added isAccessibilityScrollbar to use before casting none

Chris Guillory
Reported 2010-11-01 19:39:30 PDT
To support basic hit testing in Chromium we need to know the top document's scroll offsets. Non-top document scroll offsets are currently lower priority.
Attachments
Send document value changed on scrollbar value changed notification. (6.24 KB, patch)
2010-11-01 19:46 PDT, Chris Guillory
no flags
Proposed patch. (6.21 KB, patch)
2010-11-01 19:53 PDT, Chris Guillory
cfleizach: review-
cfleizach: commit-queue-
Added isAccessibilityScrollbar to use before casting (7.09 KB, patch)
2010-11-02 11:25 PDT, Chris Guillory
no flags
Chris Guillory
Comment 1 2010-11-01 19:46:47 PDT
Created attachment 72620 [details] Send document value changed on scrollbar value changed notification.
WebKit Review Bot
Comment 2 2010-11-01 19:50:47 PDT
Attachment 72620 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/accessibility/chromium/AXObjectCacheChromium.cpp:52: Missing space before ( in if( [whitespace/parens] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Chris Guillory
Comment 3 2010-11-01 19:53:08 PDT
Created attachment 72621 [details] Proposed patch. Fixed webkit style. Minor updates.
chris fleizach
Comment 4 2010-11-01 23:00:41 PDT
Comment on attachment 72621 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=72621&action=review looks ok otherwise. > WebCore/accessibility/chromium/AXObjectCacheChromium.cpp:52 > + if (obj->roleValue() == ScrollBarRole && notification == AXValueChanged) { i don't think obj->roleValue() is a strong enough check for doing a static_cast. If not already, I can see something like <div role="scrollbar"> in the future. You should add a method to AccessibilityScrollbar that says "isNativeScrollbar()" or something to that effect, that will allow you to safely cast.
Chris Guillory
Comment 5 2010-11-02 11:25:32 PDT
Created attachment 72704 [details] Added isAccessibilityScrollbar to use before casting
chris fleizach
Comment 6 2010-11-02 11:32:59 PDT
Comment on attachment 72704 [details] Added isAccessibilityScrollbar to use before casting r=me
WebKit Commit Bot
Comment 7 2010-11-02 16:03:19 PDT
The commit-queue encountered the following flaky tests while processing attachment 72704 [details]: media/event-attributes.html fast/workers/storage/use-same-database-in-page-and-workers.html Please file bugs against the tests. These tests were authored by dumi@chromium.org and eric.carlson@apple.com. The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 8 2010-11-02 18:20:56 PDT
Comment on attachment 72704 [details] Added isAccessibilityScrollbar to use before casting Clearing flags on attachment: 72704 Committed r71198: <http://trac.webkit.org/changeset/71198>
WebKit Commit Bot
Comment 9 2010-11-02 18:21:02 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 10 2010-11-02 19:08:45 PDT
The commit-queue encountered the following flaky tests while processing attachment 72704 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html Please file bugs against the tests. These tests were authored by dumi@chromium.org. The commit-queue is continuing to process your patch.
Note You need to log in before you can comment on or make changes to this bug.