Summary: | Send webkit accessibility notifications to Chromium | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Guillory <ctguil> | ||||||||||||||||||||||||||||||||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | abarth, cfleizach, commit-queue, dglazkov, dglazkov, dmazzoni, mihaip, webkit.review.bot | ||||||||||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Chris Guillory
2010-09-02 18:34:14 PDT
Created attachment 66451 [details]
Patch.
Attachment 66451 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ERROR: Unexpected diff format when parsing a chunk: '</ul>'
WebKit/chromium/src/AssertMatchingEnums.cpp:39: Alphabetical sorting problem. [build/include_order] [4]
WebKit/chromium/src/AssertMatchingEnums.cpp:55: Alphabetical sorting problem. [build/include_order] [4]
WebKit/chromium/src/ChromeClientImpl.cpp:722: Place brace on its own line for function definitions. [whitespace/braces] [4]
WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:492: Place brace on its own line for function definitions. [whitespace/braces] [4]
WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:495: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
Total errors found: 5 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66455 [details]
Patch
I've cleaned up some style issues and fixed up the layout test files that had an extra "AccessibilityNotification - ChildrenChanged".
Created attachment 66463 [details]
Patch
More fixes for ""AccessibilityNotification - ChildrenChanged". I've been having issues with getting expectations using new-run-webkit-tests.
Comment on attachment 66463 [details]
Patch
This looks great, but you'll need to somehow figure out how to make these test either not run or be WONTFIX in test_shell, but run and expect to pass in Chromium DRT.
Created attachment 66472 [details]
Update LayoutTests expectations
Oh yeah. Here's another patch to update the test expectations. I can put it all in one patch if that's better otherwise this can land first.
Comment on attachment 66472 [details]
Update LayoutTests expectations
Yeah, one patch would be better for archeological reasons.
Created attachment 66504 [details]
Send accessibility notifications and update chromium test expectations
Here it is in one patch.
Comment on attachment 66463 [details] Patch Cleared Dimitri Glazkov's review+ from obsolete attachment 66463 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 66504 [details] Send accessibility notifications and update chromium test expectations > > Unreviewed, updating binding-tests expectations (for changeset 66521). > Index: WebCore/accessibility/chromium/AXObjectCacheChromium.cpp > =================================================================== > --- WebCore/accessibility/chromium/AXObjectCacheChromium.cpp (revision 66453) > +++ WebCore/accessibility/chromium/AXObjectCacheChromium.cpp (working copy) > @@ -61,28 +61,13 @@ void AXObjectCache::postPlatformNotifica > > ChromeClientChromium* client = toChromeClientChromium(obj->documentFrameView()); > if (client) { You should use the early return idiom here if client == null. > - switch (notification) { > - case AXCheckedStateChanged: > + client->postAccessibilityNotification(obj, notification); > + > + // TODO: Remove after the new postAccessibilityNotification is used downstream. > + if (notification == AXCheckedStateChanged) > client->didChangeAccessibilityObjectState(obj); > - break; > - case AXChildrenChanged: > + if (notification == AXChildrenChanged) > client->didChangeAccessibilityObjectChildren(obj); > - break; looks like this should still use the switch () until its removed. > + > + // Notifies embedder about an accessibility notification. > + virtual void postAccessibilityNotification(AccessibilityObject* obj, AXObjectCache::AXNotification notification) = 0; > }; argument names not necessary here. > + > +// These values must match WebCore::AXObjectCache::AXNotification values > +enum WebAccessibilityNotification { > + ActiveDescendantChanged, > + CheckedStateChanged, > + ChildrenChanged, > + FocusedUIElementChanged, > + LayoutComplete, > + LoadComplete, > + SelectedChildrenChanged, > + SelectedTextChanged, > + ValueChanged, > + ScrolledToAnchor, > + LiveRegionChanged, > + MenuListValueChanged, > + RowCountChanged, > + RowCollapsed, > + RowExpanded, > +}; > + is there a way to avoid this duplication? it seems a hackish to have two copies that have to be kept up to date. can you export the AX notifications or the header so that you can reference those directly? > + > + // Notifies embedder about an accessibility notification. > + virtual void postAccessibilityNotification(const WebAccessibilityObject& obj, WebAccessibilityNotification notification) { } argument names not necessary > > --- WebKit/chromium/src/ChromeClientImpl.h (revision 66453) > +++ WebKit/chromium/src/ChromeClientImpl.h (working copy) > @@ -163,6 +163,8 @@ public: > virtual void popupClosed(WebCore::PopupContainer* popupContainer); > virtual void didChangeAccessibilityObjectState(WebCore::AccessibilityObject*); > virtual void didChangeAccessibilityObjectChildren(WebCore::AccessibilityObject*); > + virtual void postAccessibilityNotification( > + WebCore::AccessibilityObject* obj, WebCore::AXObjectCache::AXNotification notification); > argument names not necessary, same with the line breaks > + if (m_shell->accessibilityController()->shouldDumpAccessibilityNotifications()) { > + switch (notification) { > + case ActiveDescendantChanged: > + printf("AccessibilityNotification - ActiveDescendantChanged"); seems like you should printf "AccessibilityNotification - ") first, then just the name within the switch > + > + if (!obj.helpText().isNull()) > + printf(" - helpText:%s", obj.helpText().utf8().data()); > + seems wrong to print the help text in dumpeAccessibilityNotifications. > + virtual void postAccessibilityNotification(const WebKit::WebAccessibilityObject& obj, WebKit::WebAccessibilityNotification notification); no argument names I've made all the small changes. (In reply to comment #10) > is there a way to avoid this duplication? it seems a hackish to have two copies that have to be kept up to date. can you export the AX notifications or the header so that you can reference those directly? I'm following the way enums are brought from WebCore to WebKit that I see in AssertMatchingEnums.cpp. I'm not sure how I'd avoid maintenance. Dimitri, do you have some guidance here? > seems wrong to print the help text in dumpeAccessibilityNotifications. I'm trying to unique identify obj of the notification. What do you think if I print the id of the obj's node if it's available? (In reply to comment #11) > I've made all the small changes. > > (In reply to comment #10) > > is there a way to avoid this duplication? it seems a hackish to have two copies that have to be kept up to date. can you export the AX notifications or the header so that you can reference those directly? > I'm following the way enums are brought from WebCore to WebKit that I see in AssertMatchingEnums.cpp. I'm not sure how I'd avoid maintenance. Dimitri, do you have some guidance here? Unfortunately, we have to do this to ensure integrity of the port. It's ugly, but it's what we've got. > > > seems wrong to print the help text in dumpeAccessibilityNotifications. > I'm trying to unique identify obj of the notification. What do you think if I print the id of the obj's node if it's available?
> > seems wrong to print the help text in dumpeAccessibilityNotifications.
> I'm trying to unique identify obj of the notification. What do you think if I print the id of the obj's node if it's available?
I think that's probably a good idea
Created attachment 66936 [details]
Print id if available in dumpAccessibilityNotifications
Here's the updated patch.
Attachment 66936 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/AssertMatchingEnums.cpp:36: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 46 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66938 [details]
Removed argument names in WebViewClient.h
Attachment 66938 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/AssertMatchingEnums.cpp:36: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 46 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66944 [details]
Resolved conflict in drt_expectations.txt
Attachment 66944 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/AssertMatchingEnums.cpp:36: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 1 in 47 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #19) > Attachment 66944 [details] did not pass style-queue: > > Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 > WebKit/chromium/src/AssertMatchingEnums.cpp:36: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] > Total errors found: 1 in 47 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. waiting for style queue to pass before r+ AssertMatchingEnums.cpp does not pass style because it doesn't #include "AssertMatchingEnums.h" No such file exists. Comment on attachment 66944 [details]
Resolved conflict in drt_expectations.txt
r=me
Comment on attachment 66944 [details] Resolved conflict in drt_expectations.txt Rejecting patch 66944 from commit-queue. Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Last 500 characters of output: romium/public/WebViewClient.h M WebKit/chromium/src/AssertMatchingEnums.cpp M WebKit/chromium/src/ChromeClientImpl.cpp M WebKit/chromium/src/ChromeClientImpl.h M WebKitTools/ChangeLog M WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp M WebKitTools/DumpRenderTree/chromium/WebViewHost.h RA layer request failed: MERGE request failed on '/repository/webkit/trunk': MERGE of '/repository/webkit/trunk': 409 Conflict (http://svn.webkit.org) at /usr/local/git/libexec/git-core/git-svn line 572 Full output: http://queues.webkit.org/results/3951292 Comment on attachment 66944 [details]
Resolved conflict in drt_expectations.txt
That's a new error. Let's try that again.
Comment on attachment 66944 [details] Resolved conflict in drt_expectations.txt Rejecting patch 66944 from commit-queue. Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Last 500 characters of output: romium/public/WebViewClient.h M WebKit/chromium/src/AssertMatchingEnums.cpp M WebKit/chromium/src/ChromeClientImpl.cpp M WebKit/chromium/src/ChromeClientImpl.h M WebKitTools/ChangeLog M WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp M WebKitTools/DumpRenderTree/chromium/WebViewHost.h RA layer request failed: MERGE request failed on '/repository/webkit/trunk': MERGE of '/repository/webkit/trunk': 409 Conflict (http://svn.webkit.org) at /usr/local/git/libexec/git-core/git-svn line 572 Full output: http://queues.webkit.org/results/3935338 Created attachment 67075 [details]
Add AssertMatchingEnums.h to pass style check
The previous patch failed to commit with the commit-queue. This patch passes style.
Comment on attachment 67075 [details]
Add AssertMatchingEnums.h to pass style check
WebKitTools changelog looks busted
Created attachment 67093 [details]
Rebasing ChangeLog files
Comment on attachment 67093 [details]
Rebasing ChangeLog files
r=me
Comment on attachment 67093 [details] Rebasing ChangeLog files Rejecting patch 67093 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Chris Fleizach', u'--force']" exit_code: 1 Last 500 characters of output: enChanged.html patching file LayoutTests/platform/chromium/accessibility/post-notification-SelectedTextChanged-expected.txt patching file LayoutTests/platform/chromium/accessibility/post-notification-SelectedTextChanged.html patching file LayoutTests/platform/chromium/accessibility/post-notification-ValueChanged-expected.txt patching file LayoutTests/platform/chromium/accessibility/post-notification-ValueChanged.html patching file LayoutTests/platform/chromium/accessibility/post-notification.js Full output: http://queues.webkit.org/results/3899372 Created attachment 67209 [details] Rebased all non ChangeLog files - drt_expectations.txt conflict http://trac.webkit.org/changeset/67181 created conflict in drt_expectations.txt. Here's a rebased patch. Comment on attachment 66944 [details] Resolved conflict in drt_expectations.txt Cleared Chris Fleizach's review+ from obsolete attachment 66944 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 67093 [details] Rebasing ChangeLog files Cleared Chris Fleizach's review+ from obsolete attachment 67093 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 67209 [details] Rebased all non ChangeLog files - drt_expectations.txt conflict > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 66699) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,33 @@ > +2010-09-02 Chris Guillory <chris.guillory@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Send all accessibility notifications to Chromium > + https://bugs.webkit.org/show_bug.cgi?id=45156 > + > + Use postAccessibilityNotification to pass accessibility notification > + to chromium. > + > + Tests: platform/chromium/accessibility/post-notification-ActiveDescendantChanged.html > + platform/chromium/accessibility/post-notification-CheckedStateChanged.html > + platform/chromium/accessibility/post-notification-ChildrenChanged.html > + platform/chromium/accessibility/post-notification-FocusedUIElementChanged.html > + platform/chromium/accessibility/post-notification-LayoutComplete.html > + platform/chromium/accessibility/post-notification-LiveRegionChanged.html > + platform/chromium/accessibility/post-notification-LoadComplete.html > + platform/chromium/accessibility/post-notification-MenuListValueChanged.html > + platform/chromium/accessibility/post-notification-RowCollapsed.html > + platform/chromium/accessibility/post-notification-RowCountChanged.html > + platform/chromium/accessibility/post-notification-RowExpanded.html > + platform/chromium/accessibility/post-notification-ScrolledToAnchor.html > + platform/chromium/accessibility/post-notification-SelectedChildrenChanged.html > + platform/chromium/accessibility/post-notification-SelectedTextChanged.html > + platform/chromium/accessibility/post-notification-ValueChanged.html > + > + * accessibility/chromium/AXObjectCacheChromium.cpp: > + (WebCore::AXObjectCache::postPlatformNotification): > + * page/chromium/ChromeClientChromium.h: > + > 2010-09-02 Kinuko Yasuda <kinuko@chromium.org> > > Unreviewed, updating binding-tests expectations (for changeset 66521). > Index: WebCore/accessibility/chromium/AXObjectCacheChromium.cpp > =================================================================== > --- WebCore/accessibility/chromium/AXObjectCacheChromium.cpp (revision 66453) > +++ WebCore/accessibility/chromium/AXObjectCacheChromium.cpp (working copy) > @@ -61,28 +61,13 @@ void AXObjectCache::postPlatformNotifica > > ChromeClientChromium* client = toChromeClientChromium(obj->documentFrameView()); > if (client) { early return after null client is needed here. > + > + // Notifies embedder about an accessibility notification. > + virtual void postAccessibilityNotification(AccessibilityObject* obj, AXObjectCache::AXNotification notification) = 0; > }; no argument names > + > + // Notifies embedder about an accessibility notification. > + virtual void postAccessibilityNotification(const WebAccessibilityObject& obj, WebAccessibilityNotification notification) { } > no argument names > +COMPILE_ASSERT_MATCHING_ENUM(ActiveDescendantChanged, AXObjectCache::AXActiveDescendantChanged); > +COMPILE_ASSERT_MATCHING_ENUM(CheckedStateChanged, AXObjectCache::AXCheckedStateChanged); > +COMPILE_ASSERT_MATCHING_ENUM(ChildrenChanged, AXObjectCache::AXChildrenChanged); > +COMPILE_ASSERT_MATCHING_ENUM(FocusedUIElementChanged, AXObjectCache::AXFocusedUIElementChanged); > +COMPILE_ASSERT_MATCHING_ENUM(LayoutComplete, AXObjectCache::AXLayoutComplete); > +COMPILE_ASSERT_MATCHING_ENUM(LoadComplete, AXObjectCache::AXLoadComplete); > +COMPILE_ASSERT_MATCHING_ENUM(SelectedChildrenChanged, AXObjectCache::AXSelectedChildrenChanged); > +COMPILE_ASSERT_MATCHING_ENUM(SelectedTextChanged, AXObjectCache::AXSelectedTextChanged); > +COMPILE_ASSERT_MATCHING_ENUM(ValueChanged, AXObjectCache::AXValueChanged); > +COMPILE_ASSERT_MATCHING_ENUM(ScrolledToAnchor, AXObjectCache::AXScrolledToAnchor); > +COMPILE_ASSERT_MATCHING_ENUM(LiveRegionChanged, AXObjectCache::AXLiveRegionChanged); > +COMPILE_ASSERT_MATCHING_ENUM(MenuListValueChanged, AXObjectCache::AXMenuListValueChanged); > +COMPILE_ASSERT_MATCHING_ENUM(RowCountChanged, AXObjectCache::AXRowCountChanged); > +COMPILE_ASSERT_MATCHING_ENUM(RowCollapsed, AXObjectCache::AXRowCollapsed); > +COMPILE_ASSERT_MATCHING_ENUM(RowExpanded, AXObjectCache::AXRowExpanded); > + going to re-open this again.... can you not do a runtime lookup of these keys? that way as new ones are added, other platforms aren't responsible for modifying this list and chromium only needs to lookup the ones it needs? just something like WebNotification noti = 0; switch (WebCore::notification) { case RowExpanded: noti = WebNotificationRowExpanded; break; } > virtual void didChangeAccessibilityObjectState(WebCore::AccessibilityObject*); > virtual void didChangeAccessibilityObjectChildren(WebCore::AccessibilityObject*); > + virtual void postAccessibilityNotification( > + WebCore::AccessibilityObject* obj, WebCore::AXObjectCache::AXNotification notification); > no argument names > // ChromeClientImpl: > void setCursor(const WebCursorInfo& cursor); > Index: WebKitTools/ChangeLog > =================================================================== > --- WebKitTools/ChangeLog (revision 66699) > +++ WebKitTools/ChangeLog (working copy) > @@ -1,3 +1,14 @@ > +2010-09-02 Chris Guillory <chris.guillory@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Send all accessibility notifications to Chromium > + https://bugs.webkit.org/show_bug.cgi?id=45156 > + > + * DumpRenderTree/chromium/WebViewHost.cpp: > + (WebViewHost::postAccessibilityNotification): > + * DumpRenderTree/chromium/WebViewHost.h: > + > 2010-09-02 Steve Block <steveblock@google.com> > > Reviewed by Adam Barth. > Index: WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp > =================================================================== > --- WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp (revision 66453) > +++ WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp (working copy) > @@ -489,10 +489,62 @@ void WebViewHost::focusAccessibilityObje > m_shell->accessibilityController()->setFocusedElement(object); > } > > -void WebViewHost::didChangeAccessibilityObjectChildren(const WebAccessibilityObject& object) > +void WebViewHost::postAccessibilityNotification(const WebAccessibilityObject& obj, WebAccessibilityNotification notification) > { > - if (m_shell->accessibilityController()->shouldDumpAccessibilityNotifications()) > - printf("didChangeAccessibilityObjectChildren - new count: %d\n", object.childCount()); > + if (m_shell->accessibilityController()->shouldDumpAccessibilityNotifications()) { > + switch (notification) { > + case ActiveDescendantChanged: > + printf("AccessibilityNotification - ActiveDescendantChanged"); > + break; > + case CheckedStateChanged: > + printf("AccessibilityNotification - CheckedStateChanged"); > + break; > + case ChildrenChanged: > + printf("AccessibilityNotification - ChildrenChanged"); > + break; > + case FocusedUIElementChanged: > + printf("AccessibilityNotification - FocusedUIElementChanged"); > + break; > + case LayoutComplete: > + printf("AccessibilityNotification - LayoutComplete"); > + break; > + case LoadComplete: > + printf("AccessibilityNotification - LoadComplete"); > + break; > + case SelectedChildrenChanged: > + printf("AccessibilityNotification - SelectedChildrenChanged"); > + break; > + case SelectedTextChanged: > + printf("AccessibilityNotification - SelectedTextChanged"); > + break; > + case ValueChanged: > + printf("AccessibilityNotification - ValueChanged"); > + break; > + case ScrolledToAnchor: > + printf("AccessibilityNotification - ScrolledToAnchor"); > + break; > + case LiveRegionChanged: > + printf("AccessibilityNotification - LiveRegionChanged"); > + break; > + case MenuListValueChanged: > + printf("AccessibilityNotification - MenuListValueChanged"); > + break; > + case RowCountChanged: > + printf("AccessibilityNotification - RowCountChanged"); > + break; > + case RowCollapsed: > + printf("AccessibilityNotification - RowCollapsed"); > + break; > + case RowExpanded: > + printf("AccessibilityNotification - RowExpanded"); > + break; > + } > + > + if (!obj.helpText().isNull()) > + printf(" - helpText:%s", obj.helpText().utf8().data()); > + didn't we fix this before? > going to re-open this again.... > can you not do a runtime lookup of these keys? that way as new ones are added, other platforms aren't responsible for modifying this list and > chromium only needs to lookup the ones it needs? > > just something like > WebNotification noti = 0; > switch (WebCore::notification) { > case RowExpanded: > noti = WebNotificationRowExpanded; > break; > } I see what you're saying now. We should be able to assign the values directly in the enum definition. > didn't we fix this before? We fixed a lot of this before. I must have uploaded an old patch by mistake. Created attachment 67375 [details]
Not using AssertMatchingEnums. Rebased.
Created attachment 67378 [details]
Some of the layout test results had an extra newline
Created attachment 67464 [details]
Recreating patch to fix patch errors.
Comment on attachment 67464 [details]
Recreating patch to fix patch errors.
r=me
Comment on attachment 67464 [details] Recreating patch to fix patch errors. Rejecting patch 67464 from commit-queue. Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Last 500 characters of output: eLog M WebKit/chromium/WebKit.gyp M WebKit/chromium/public/WebViewClient.h M WebKit/chromium/src/ChromeClientImpl.cpp M WebKit/chromium/src/ChromeClientImpl.h M WebKitTools/ChangeLog M WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp M WebKitTools/DumpRenderTree/chromium/WebViewHost.h RA layer request failed: MERGE request failed on '/repository/webkit/trunk': MERGE of '/repository/webkit/trunk': 409 Conflict (http://svn.webkit.org) at /usr/local/git/libexec/git-core/git-svn line 572 Full output: http://queues.webkit.org/results/4002007 Second time this commit-queue error has occurred with this patch. I get no conflict like this when I update my svn client. Can this patch be submitted manually? (In reply to comment #41) > Second time this commit-queue error has occurred with this patch. I get no conflict like this when I update my svn client. Can this patch be submitted manually? The following files contain tab characters: trunk/LayoutTests/platform/chromium/accessibility/post-notification-MenuListValueChanged.html Please use spaces instead to indent. If you must commit a file with tabs, use svn propset to set the "allow-tabs" property. Committed r67498: <http://trac.webkit.org/changeset/67498> This causes a lot of tests to fail with the Chromium DRT, for example editing/undo/5738768.html: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fundo%2F5738768.html&upstreamWebKit=true It looks like a lot of AccessibilityNotification being printed out, caused by a stray print left in WebViewHost::postAccessibilityNotification. Created attachment 67632 [details]
Patch
Comment on attachment 67632 [details] Patch Clearing flags on attachment: 67632 Committed r67527: <http://trac.webkit.org/changeset/67527> All reviewed patches have been landed. Closing bug. Created attachment 68187 [details]
Send remaining webkit notifications to Chromium
Need remaining notifications. Comment on attachment 68187 [details] Send remaining webkit notifications to Chromium View in context: https://bugs.webkit.org/attachment.cgi?id=68187&action=review > WebCore/ChangeLog:6 > + https://bugs.webkit.org/show_bug.cgi?id=45156 comment should end with a period > WebCore/accessibility/chromium/AXObjectCacheChromium.cpp:88 > + case AXRowExpanded: these cases should be alphabetized > WebCore/accessibility/chromium/AXObjectCacheChromium.cpp:109 > + this ASSERT might not be correct. for example, this is possible <div role="group" aria-hidden="true" tabindex="0">test</div> where the div should be ignored, but it can be focused > WebCore/editing/chromium/SelectionControllerChromium.cpp:42 > + seems like you don't need to access document until it's inside the if block, so there's no need to declare out here Created attachment 68259 [details]
Update patch
Thanks. Here an updated patch.
Comment on attachment 68259 [details]
Update patch
r=me
Comment on attachment 68259 [details] Update patch Clearing flags on attachment: 68259 Committed r67982: <http://trac.webkit.org/changeset/67982> All reviewed patches have been landed. Closing bug. Created attachment 68389 [details] Reland patch The previous revision http://trac.webkit.org/changeset/67982 was reverted due to bug 46237. Marking unconfirmed. Comment on attachment 68389 [details]
Reland patch
r=me
Comment on attachment 68389 [details] Reland patch Clearing flags on attachment: 68389 Committed r68240: <http://trac.webkit.org/changeset/68240> All reviewed patches have been landed. Closing bug. |