Class DocumentMarker::MarkerTypes represents bitmask of marker types and duplicates most of the functionality of OptionSet. Unlike OptionSet, DocumentMarker::MarkerTypes does not support iteration and hence code, such as AlternativeTextController::applyAlternativeTextToRange(), uses Vector<DocumentMarker::MarkerType> to represent a set of marker types. We can use OptionSet to support both uses cases in a space- and time- efficient manner.
Created attachment 315713 [details] Patch
Comment on attachment 315713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315713&action=review > Source/WebCore/dom/DocumentMarker.h:164 > + AcceptedCandidate > + , Autocorrected > + , CorrectionIndicator > + , DeletedAutocorrection > + , DictationAlternatives > + , Grammar > + , RejectedCorrection > + , Replacement > + , SpellCheckingExemption > + , Spelling > + , TextMatch > +#if ENABLE(TELEPHONE_NUMBER_DETECTION) > + , TelephoneNumber > +#endif > +#if PLATFORM(IOS) > + , DictationPhraseWithAlternatives > + , DictationResult > +#endif > + , DraggedContent I would like to see this sorted alphabetically (including "DraggedContent", but not including the conditional items. And we should use trailing commas instead of leading commas, with the extra one at the end that C++ allows in initializer lists. That way we don’t have to have one line different from the others. > Source/WebCore/editing/Editor.cpp:2752 > + DocumentMarker::Spelling, DocumentMarker::CorrectionIndicator, DocumentMarker::SpellCheckingExemption, > + DocumentMarker::DictationAlternatives > +#if !PLATFORM(IOS) > + , DocumentMarker::Grammar > #endif Trailing commas after each item is a cleaner way to do this since you can have an extra comma.
You will have to rebase and merge with r219595 since it overlaps some functions you changed.
(In reply to Darin Adler from comment #2) > > Source/WebCore/dom/DocumentMarker.h:164 > > + AcceptedCandidate > > + , Autocorrected > > + , CorrectionIndicator > > + , DeletedAutocorrection > > + , DictationAlternatives > > + , Grammar > > + , RejectedCorrection > > + , Replacement > > + , SpellCheckingExemption > > + , Spelling > > + , TextMatch > > +#if ENABLE(TELEPHONE_NUMBER_DETECTION) > > + , TelephoneNumber > > +#endif > > +#if PLATFORM(IOS) > > + , DictationPhraseWithAlternatives > > + , DictationResult > > +#endif > > + , DraggedContent > > I would like to see this sorted alphabetically (including "DraggedContent", > but not including the conditional items. And we should use trailing commas > instead of leading commas, with the extra one at the end that C++ allows in > initializer lists. That way we don’t have to have one line different from > the others. > Will fix. > > Source/WebCore/editing/Editor.cpp:2752 > > + DocumentMarker::Spelling, DocumentMarker::CorrectionIndicator, DocumentMarker::SpellCheckingExemption, > > + DocumentMarker::DictationAlternatives > > +#if !PLATFORM(IOS) > > + , DocumentMarker::Grammar > > #endif > > Trailing commas after each item is a cleaner way to do this since you can > have an extra comma. Will fix.
Committed r219597: <http://trac.webkit.org/changeset/219597>
This change seems to break WinCairo bot WebFrame.cpp.obj : error LNK2019: unresolved external symbol "public: void __cdecl WebCore::DocumentMarkerController::removeMarkers(class WebCore::DocumentMarker::MarkerTypes)" (?removeMarkers@DocumentMarkerController@WebCore@@QEAAXVMarkerTypes@DocumentMarker@2@@Z) referenced in function "public: void __cdecl WebFrame::unmarkAllMisspellings(void)" (?unmarkAllMisspellings@WebFrame@@QEAAXXZ) https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/4428/steps/compile-webkit/logs/stdio
That looks like a build dependency error that we would get if we tried to link WebKitLegacy against WebCore and had not recompiled WebKitLegacy, rather than a mistake in this patch. Could force the WinCairo bot to do a clean build, or try to figure out how to fix the build system used by WinCairo to correctly understand this dependency.