[Webkit-unassigned] [Bug 47630] reversion bubble in WebViews

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47630





--- Comment #3 from jpu at apple.com  2010-11-04 11:56:07 PST ---
(In reply to comment #2)
> (From update of attachment 72904 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72904&action=review
> 
> Generally good, but enough comments to warrant a second round.

Thanks for the review. I will make change as you suggest except two places that I have replied to your comment below.

> 
> > WebCore/ChangeLog:18
> > +        1. On Mac OS X, the result of spell checking is partly determined by past user usage. We can't
> > +           realiably generating test cases until we can disable user custom data during spell checking.
> 
> Is there a way to set up the environment for DumpRenderTree such that it won’t be affected by user data?

An SPI has been recently added to AppKit to disable user data. I suggest we hook up DumpRenderTree with said SPI in another patch. However, to make the automated test fast, we still need to be able to fake the delay.


> > WebCore/editing/CorrectionPanelInfo.h:58
> > +enum CorrectionWasRejectedOrNot { CorrectionWasNotRejected, CorrectionWasRejected };
> 
> I wonder if “Accepted” would be better than “Not Rejected”.

The reason for this more confusing naming is due to the three different ways with which the correction panel can be dismissed. 
1. Reject: when user press ESC key or click on the cross button on the panel.
2. Ignore: when user continues typing.
3. Accept: when user types whitespace or punctuation mark.
This argument is used to distinguish case 1 from case 2 and 3, hence the name. Maybe I should use a enum type with three values, even though the handling of two of those values would be the same. That would make explicit the concept I just described. Thought?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.