Bug 57800

Summary: [Mac] When autocorrection occurs without showing correction panel, WebCore need to post accessibility notification.
Product: WebKit Reporter: Jia Pu <jiapu.mail>
Component: New BugsAssignee: Jia Pu <jiapu.mail>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ddavidso, dglazkov, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
Attachments:
Description Flags
Patch (v1)
none
Patch (v2)
none
Patch (v4) none

Description Jia Pu 2011-04-04 16:10:37 PDT
<rdar://problem/9218223>
Comment 1 Jia Pu 2011-04-04 17:11:31 PDT
Created attachment 88167 [details]
Patch (v1)
Comment 2 Darin Adler 2011-04-04 17:24:58 PDT
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);
Comment 3 WebKit Review Bot 2011-04-04 18:15:14 PDT
Attachment 88167 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8336085
Comment 4 Jia Pu 2011-04-05 09:03:46 PDT
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.
Comment 5 Darin Adler 2011-04-05 09:30:36 PDT
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.
Comment 6 Jia Pu 2011-04-05 10:26:21 PDT
Created attachment 88271 [details]
Patch (v4)
Comment 7 Jia Pu 2011-04-05 15:32:16 PDT
(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.
Comment 8 WebKit Commit Bot 2011-04-06 07:57:36 PDT
Comment on attachment 88271 [details]
Patch (v4)

Clearing flags on attachment: 88271

Committed r83049: <http://trac.webkit.org/changeset/83049>
Comment 9 WebKit Commit Bot 2011-04-06 07:57:46 PDT
All reviewed patches have been landed.  Closing bug.