Bug 48817 - Chromium: Propagate a document value changed notification on scroll.
: Chromium: Propagate a document value changed notification on scroll.
Status: RESOLVED FIXED
: WebKit
Accessibility
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-11-01 19:39 PST by
Modified: 2010-11-02 19:08 PST (History)


Attachments
Send document value changed on scrollbar value changed notification. (6.24 KB, patch)
2010-11-01 19:46 PST, Chris Guillory
no flags Review Patch | Details | Formatted Diff | Diff
Proposed patch. (6.21 KB, patch)
2010-11-01 19:53 PST, Chris Guillory
cfleizach: review-
cfleizach: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Added isAccessibilityScrollbar to use before casting (7.09 KB, patch)
2010-11-02 11:25 PST, Chris Guillory
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2010-11-01 19:39:30 PST
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.
------- Comment #1 From 2010-11-01 19:46:47 PST -------
Created an attachment (id=72620) [details]
Send document value changed on scrollbar value changed notification.
------- Comment #2 From 2010-11-01 19:50:47 PST -------
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.
------- Comment #3 From 2010-11-01 19:53:08 PST -------
Created an attachment (id=72621) [details]
Proposed patch.

Fixed webkit style. Minor updates.
------- Comment #4 From 2010-11-01 23:00:41 PST -------
(From update of attachment 72621 [details])
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.
------- Comment #5 From 2010-11-02 11:25:32 PST -------
Created an attachment (id=72704) [details]
Added isAccessibilityScrollbar to use before casting
------- Comment #6 From 2010-11-02 11:32:59 PST -------
(From update of attachment 72704 [details])
r=me
------- Comment #7 From 2010-11-02 16:03:19 PST -------
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.
------- Comment #8 From 2010-11-02 18:20:56 PST -------
(From update of attachment 72704 [details])
Clearing flags on attachment: 72704

Committed r71198: <http://trac.webkit.org/changeset/71198>
------- Comment #9 From 2010-11-02 18:21:02 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #10 From 2010-11-02 19:08:45 PST -------
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.