[webkit-reviews] review denied: [Bug 48078] Editing a word with misspell or autocorrection underline should remove the underline when the editing changes the word. : [Attachment 72204] Proposed patch (v4)

[bugzilla-daemon at webkit.org]

mitz at webkit.org has denied jpu at apple.com's request for review:
Bug 48078: Editing a word with misspell or autocorrection underline should
remove the underline when the editing changes the word.
https://bugs.webkit.org/show_bug.cgi?id=48078

Attachment 72204: Proposed patch (v4)
https://bugs.webkit.org/attachment.cgi?id=72204&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=72204&action=review

This is very close but I still have a few comments.

> WebCore/dom/DocumentMarker.h:36
> +    // Changed MarkerType from regular enum to bit mask to make searching
for multiple types easier.

This comment is appropriate for a change log, but it will not make sense to
anyone reading this file in the future. Just remove it.

> WebCore/dom/DocumentMarker.h:46
>      MarkerType type;

You should also typedef unsigned MarkerTypes.

> WebCore/dom/DocumentMarkerController.cpp:534
> +bool DocumentMarkerController::hasMarkers(Range* range, unsigned
markerTypes)

Use the MarkerTypes typedef here.

> WebCore/editing/Editor.cpp:2977
> +    static const unsigned markerTypesToRemove = DocumentMarker::Spelling |
DocumentMarker::CorrectionIndicator;

“static” doesn’t do anything here. I’d not use a variable at all for this, but
that’s just a matter of taste.