[Webkit-unassigned] [Bug 48078] Editing a word with misspell or autocorrection underline should remove the underline when the editing changes the word.

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from jpu at apple.com  2010-10-28 05:53:06 PST ---
(In reply to comment #9)
> (From update of attachment 72140 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72140&action=review
> 
> r- because of config.h and the problem in hasMarker().
> 
> > WebCore/config.h:176
> > +// Some platforms provide UI for suggesting autocorrection.
> > +#define SUPPORT_AUTOCORRECTION_PANEL 0
> > +// Some platforms use spelling and autocorrection markers to provide visual cue.
> > +// On such platform, if word with marker is edited, we need to remove the marker.
> > +#define REMOVE_MARKERS_UPON_EDITING 0
> > +
> >  #if PLATFORM(MAC)
> >  // New theme
> >  #define WTF_USE_NEW_THEME 1
> > +#if !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD)
> > +#define SUPPORT_AUTOCORRECTION_PANEL 1
> > +#define REMOVE_MARKERS_UPON_EDITING 1
> > +#endif // !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD)
> >  #endif // PLATFORM(MAC)
> 
> config.h isn’t the right place for defining these things. These are only relevant to one or two files in editing and should probably be defined in some header in editing (that’s included by those files).

Moving these macros to Editor.h. 

> > WebCore/dom/DocumentMarkerController.cpp:546
> > +    Node* startContainer = range->startContainer(ec);
> > +    if (ec)
> > +        return false;
> > +    Node* endContainer = range->endContainer(ec);
> > +    if (ec)
> > +        return false;
> 
> I wonder if we can guarantee that the range has start and end containers and replace those returns with assertions.

This is indeed guaranteed at call site. Using assertions instead.

> > WebCore/editing/Editor.cpp:2977
> > +    Vector<DocumentMarker::MarkerType> markerTypeToRemove;
> 
> This should be called markerTypesToRemove and initialized with a size of 2:
>     Vector<DocumentMarker::MarkerType> markerTypesToRemove(2);
> then instead of appending you can assign to the 0th and 1st elements.
> Better yet, make this a static and make a helper function to initialize this once.
> 
>     const Vector<DocumentMarker::MarkerType>& markerTypesToRemove = createMarkerTypesToRemove();
> 
> Another idea is to use a bit mask instead of a vector. Checking if the marker type is in the vector is slower than testing a bit mask.

Changing DocumentMarker::MarkerType from regular enum to bit masks.

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