WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
91073
Add accessible for QWebView.
https://bugs.webkit.org/show_bug.cgi?id=91073
Summary
Add accessible for QWebView.
Frederik Gladhorn
Reported
2012-07-12 04:30:27 PDT
Add accessible for QWebView.
Attachments
Patch
(12.38 KB, patch)
2012-07-12 04:33 PDT
,
Frederik Gladhorn
no flags
Details
Formatted Diff
Diff
Patch
(12.46 KB, patch)
2012-07-12 05:53 PDT
,
Frederik Gladhorn
no flags
Details
Formatted Diff
Diff
Patch
(11.90 KB, patch)
2012-07-13 07:29 PDT
,
Frederik Gladhorn
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Frederik Gladhorn
Comment 1
2012-07-12 04:33:12 PDT
Created
attachment 151908
[details]
Patch
WebKit Review Bot
Comment 2
2012-07-12 04:35:37 PDT
Attachment 151908
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/..." exit_code: 1 Source/WebKit/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/qt/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/qt/Api/qwebviewaccessible_p.h:23: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Frederik Gladhorn
Comment 3
2012-07-12 05:53:19 PDT
Created
attachment 151926
[details]
Patch
Simon Hausmann
Comment 4
2012-07-13 04:22:51 PDT
Comment on
attachment 151926
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=151926&action=review
A few comments. I like the approach of doing this in small steps, this being the first one. Let's do one more round because of the coding style and the accesslbeInterfaceFactory function placement. I think ideally it should be static somewhere, alternatively it should be namespaced.
> Source/WebKit/qt/Api/qwebviewaccessible.cpp:28 > +QAccessibleInterface* accessibleInterfaceFactory(const QString& key, QObject* object)
Would it make perhaps sense to move this into qwebpage.cpp where it is called/used and make it a static function there?
> Source/WebKit/qt/Api/qwebviewaccessible.cpp:32 > + if (QWebPage *page = qobject_cast<QWebPage*>(object))
Coding style nitpick, * placement :)
> Source/WebKit/qt/Api/qwebviewaccessible.cpp:35 > + if (QWebView *view = qobject_cast<QWebView*>(object))
Ditto.
> Source/WebKit/qt/Api/qwebviewaccessible.cpp:38 > + if (QWebFrame *frame = qobject_cast<QWebFrame*>(object))
Ditto.
> Source/WebKit/qt/Api/qwebviewaccessible.cpp:71 > +// WebCore::Document* document = frame()->d->frame->document(); > +// if (!document || !document->axObjectCache()) > +// return 0; > +// WebCore::AccessibilityObject* rootAccessible = document->axObjectCache()->rootObject(); > +// return rootAccessible ? 1 : 0;
We probably shouldn't land the code that is commented out.
Frederik Gladhorn
Comment 5
2012-07-13 07:29:39 PDT
Created
attachment 152256
[details]
Patch
Frederik Gladhorn
Comment 6
2012-07-13 07:30:09 PDT
Thanks for the feedback :) I hope I've addressed the issues mentioned.
WebKit Review Bot
Comment 7
2012-07-16 00:46:04 PDT
Comment on
attachment 152256
[details]
Patch Clearing flags on attachment: 152256 Committed
r122702
: <
http://trac.webkit.org/changeset/122702
>
WebKit Review Bot
Comment 8
2012-07-16 00:46:08 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug