Bug 57800 – [Mac] When autocorrection occurs without showing correction panel, WebCore need to post accessibility notification.

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

[Jia Pu]

<rdar://problem/9218223>

[Jia Pu]

Created attachment 88167 [details]
Patch (v1)

[Darin Adler]

Comment on attachment 88167 [details]
Patch (v1)

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

> Source/WebCore/editing/Editor.cpp:2417
> +                        Element* rootEditable = m_frame->selection()->selection().rootEditableElement();
> +                        if (rootEditable)
> +                            m_frame->document()->axObjectCache()->postNotification(rootEditable->renderer(), AXObjectCache::AXAutocorrectionOccured, true);

It’s often better style to write it like this:

    if (Element* root = m_frame->selection()->selection().rootEditableElement())
        m_frame->document()->axObjectCache()->postNotification(root->renderer(), AXObjectCache::AXAutocorrectionOccured, true);

[WebKit Review Bot]

Attachment 88167 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8336085

[Jia Pu]

Created attachment 88243 [details]
Patch (v2)

Fixed build failure on chromium. Changed the location from where we post notification, so that text replacement other than autocorrection also generates notification.

[Darin Adler]

Comment on attachment 88243 [details]
Patch (v2)

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

> Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:77
> +#if !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD)
> +        case AXAutocorrectionOccured:
> +            macNotification = @"AXAutocorrectionOccurred";
> +            break;
> +#endif

If the enum value exists, we probably need a case for it on all versions, not just versions newer than Snow Leopard. We could do ASSERT_NOT_REACHED on those other versions, but we will still want the case because you did not put an #if around the enum value.

[Jia Pu]

Created attachment 88271 [details]
Patch (v4)

[Jia Pu]

(In reply to comment #5)
> (From update of attachment 88243 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88243&action=review
> 
> > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:77
> > +#if !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD)
> > +        case AXAutocorrectionOccured:
> > +            macNotification = @"AXAutocorrectionOccurred";
> > +            break;
> > +#endif
> 
> If the enum value exists, we probably need a case for it on all versions, not just versions newer than Snow Leopard. We could do ASSERT_NOT_REACHED on those other versions, but we will still want the case because you did not put an #if around the enum value.

I have made slight change to make sure this builds on SL. Darin, could you take a look and r+ it if it's OK?Thanks.

[WebKit Commit Bot]

Comment on attachment 88271 [details]
Patch (v4)

Clearing flags on attachment: 88271

Committed r83049: <http://trac.webkit.org/changeset/83049>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.