WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
31525
WAI-ARIA roles not supported on image map <area>
https://bugs.webkit.org/show_bug.cgi?id=31525
Summary
WAI-ARIA roles not supported on image map <area>
chris fleizach
Reported
2009-11-15 18:26:42 PST
WAI-ARIA roles not supported on image map <area>
Attachments
patch
(14.97 KB, patch)
2009-11-17 23:20 PST
,
chris fleizach
darin
: review-
Details
Formatted Diff
Diff
patch
(13.26 KB, patch)
2009-11-18 21:31 PST
,
chris fleizach
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
chris fleizach
Comment 1
2009-11-17 23:20:58 PST
Created
attachment 43409
[details]
patch
Darin Adler
Comment 2
2009-11-18 10:06:08 PST
Comment on
attachment 43409
[details]
patch
> + String ariaRole = m_areaElement->getAttribute(roleAttr);
It's usually a slightly better idiom to put fetched attributes into const AtomicString& local variables instead of String, because it avoids a little bit of reference count churn.
> +static const ARIARoleMap& createARIARoleMap()
I think it's cleaner for a create function to return an ARIARoleMap* rather than a const ARIARoleMap&. I realize you're just moving this code.
> +AccessibilityRole AccessibilityObject::ariaRoleToWebCoreRole(String& value)
The argument should be const String&, not String&.
> + ASSERT(!value.isEmpty() && !value.isNull());
Normally we don't use && in assertions. Instead we do multiple assertions in such cases; that way we know which one failed. Also, all null strings are also empty, so the only reason to have both assertions is if you want to know from the assertion which is which, and if so, the null assertion has to come first.
> - AccessibilityRole role = ariaRoleToWebCoreRole(ariaRole); > + AccessibilityRole role = AccessibilityObject::ariaRoleToWebCoreRole(ariaRole);
Do you really need to change this? Since AccessibilityRenderObject is derived from AccessibilityObject I'd expect this to already be in scope. review- because of the String& thing -- everything else is too minor to matter, but that is not good
chris fleizach
Comment 3
2009-11-18 21:31:02 PST
Created
attachment 43483
[details]
patch made all of darin's suggestions
chris fleizach
Comment 4
2009-11-19 10:58:39 PST
adding to commit queue, cuz the code is elsewhere
WebKit Commit Bot
Comment 5
2009-11-19 11:28:34 PST
Comment on
attachment 43483
[details]
patch Clearing flags on attachment: 43483 Committed
r51195
: <
http://trac.webkit.org/changeset/51195
>
WebKit Commit Bot
Comment 6
2009-11-19 11:28:40 PST
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