Bug 204233 – AX: Update style of lambda in isolated tree

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

Update style to remove unneeded & capture
Remove Options<AXID> which is unnecessary

[Radar WebKit Bug Importer]

<rdar://problem/57232085>

[chris fleizach]

Created attachment 383628 [details]
patch

[Darin Adler]

Comment on attachment 383628 [details]
patch

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

Change looks great but I am not sure I understand why the code was working.

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:98
> -        setProperty(AXPropertyName::MathLineThickness, object.mathLineThickness());Ã¥Ã
> +        setProperty(AXPropertyName::MathLineThickness, object.mathLineThickness());

Wow, what was that?

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:195
> -        [&] (Optional<AXID> typedValue) {
> -            if (!typedValue)
> -                return InvalidAXID;
> -        return typedValue.value();
> -        },
> +        [] (AXID& typedValue) { return typedValue; },

This looks great.

I don’t understand why the old code even compiled, though! And if it did compile, why did it ever work. There’s no Optional<AXID> in the Variant.

Do we have test coverage for this?

[chris fleizach]

(In reply to Darin Adler from comment #3)
> Comment on attachment 383628 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=383628&action=review
> 
> Change looks great but I am not sure I understand why the code was working.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:98
> > -        setProperty(AXPropertyName::MathLineThickness, object.mathLineThickness());Ã¥Ã
> > +        setProperty(AXPropertyName::MathLineThickness, object.mathLineThickness());
> 
> Wow, what was that?

no idea... some errant character slipped in at the last second

> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedTreeNode.cpp:195
> > -        [&] (Optional<AXID> typedValue) {
> > -            if (!typedValue)
> > -                return InvalidAXID;
> > -        return typedValue.value();
> > -        },
> > +        [] (AXID& typedValue) { return typedValue; },
> 
> This looks great.
> 
> I don’t understand why the old code even compiled, though! And if it did
> compile, why did it ever work. There’s no Optional<AXID> in the Variant.
> 

Definitely compiled and appeared to work (unless I screwed up badly).

> Do we have test coverage for this?

Andres is working on setting up the test structure for this now. 
https://bugs.webkit.org/show_bug.cgi?id=204226

Thanks!

[WebKit Commit Bot]

Comment on attachment 383628 [details]
patch

Clearing flags on attachment: 383628

Committed r252515: <https://trac.webkit.org/changeset/252515>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.