Bug 65086 – Fix AtomicString vs. String usage in accessibility code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Patrick R. Gansterer]

Fix AtomicString vs. String usage in accessibility code

[Patrick R. Gansterer]

Created attachment 101835 [details]
Patch

[Darin Adler]

Comment on attachment 101835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=101835&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1237
> -        String idName = idVector[i];
> +        AtomicString idName(idVector[i]);
>          Element* idElement = scope->getElementById(idName);

There’s no value to having a local variable here. It should just be getElementById(idVector[i]).

[Patrick R. Gansterer]

(In reply to comment #2)
> (From update of attachment 101835 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101835&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1237
> > -        String idName = idVector[i];
> > +        AtomicString idName(idVector[i]);
> >          Element* idElement = scope->getElementById(idName);
> 
> There’s no value to having a local variable here. It should just be getElementById(idVector[i]).
We need to make a AtomicString out of String, to make the explicit cast work (I should mention that in the ChangeLog... sorry).
The alternative line would be getElementById(AtomicString(idVector[i])).
An other option is to add a split operator which generates a list of AtomicString, but I don't see much value in this.

[Darin Adler]

(In reply to comment #3)
> An other option is to add a split operator which generates a list of AtomicString, but I don't see much value in this.

There’s a little bit of value in that.

[WebKit Review Bot]

Comment on attachment 101835 [details]
Patch

Clearing flags on attachment: 101835

Committed r91718: <http://trac.webkit.org/changeset/91718>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.