WebKit 1 should support action menus for non-editable text. rdar://problem/18877483
Created attachment 241265 [details] Patch
Attachment 241265 [details] did not pass style-queue: ERROR: Source/WebKit/mac/WebView/WebActionMenuController.mm:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKit/mac/WebView/WebActionMenuController.mm:320: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit/mac/WebView/WebActionMenuController.mm:409: Missing space before ( in if( [whitespace/parens] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:40: The parameter name "position" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:40: The parameter name "range" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:41: The parameter name "position" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:41: The parameter name "selection" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:43: The parameter name "position" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:44: The parameter name "selection" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.h:45: The parameter name "hitTestResult" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/editing/DictionaryLookup.mm:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 11 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 241265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241265&action=review Lots of stylebot comments. > Source/WebCore/editing/DictionaryLookup.h:43 > +bool isPositionInRange(const VisiblePosition& position, Range* range); > +bool shouldUseSelection(const VisiblePosition& position, const VisibleSelection& selection); > + > +PassRefPtr<Range> rangeExpandedAroundPositionByCharacters(const VisiblePosition& position, int numberOfCharactersToExpand); isPositionInRange and rangeExpandedAroundPositionByCharacters should probably be on VisiblePosition or Range or something, they're pretty generically useful (maybe there's already something there that we can replace these with?) > Source/WebCore/editing/DictionaryLookup.mm:29 > +#include "Document.h" all of these should be 'import' > Source/WebCore/editing/DictionaryLookup.mm:77 > +PassRefPtr<Range> rangeExpandedAroundPositionByCharacters(const VisiblePosition& position, int numberOfCharactersToExpand) Remind me to make use of this in Editor, where this code was originally stolen from. > Source/WebCore/editing/DictionaryLookup.mm:116 > + wkExtractWordDefinitionTokenRangeFromContextualString(fullPlainTextString, rangeToPass, options); We should probably ditch the WKSI wrappers and add some LookUp SPI headers. > Source/WebKit/mac/WebView/WebActionMenuController.mm:297 > +static DictionaryPopupInfo performDictionaryLookupForRange(Frame* frame, Range& range, NSDictionary *options) Can we put more of this in the WebCore?
(In reply to comment #3) > Comment on attachment 241265 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241265&action=review > > Lots of stylebot comments. > Fixed! > > Source/WebCore/editing/DictionaryLookup.h:43 > > +bool isPositionInRange(const VisiblePosition& position, Range* range); > > +bool shouldUseSelection(const VisiblePosition& position, const VisibleSelection& selection); > > + > > +PassRefPtr<Range> rangeExpandedAroundPositionByCharacters(const VisiblePosition& position, int numberOfCharactersToExpand); > > isPositionInRange and rangeExpandedAroundPositionByCharacters should > probably be on VisiblePosition or Range or something, they're pretty > generically useful (maybe there's already something there that we can > replace these with?) > Agreed. I filed https://bugs.webkit.org/show_bug.cgi?id=138567 > > Source/WebCore/editing/DictionaryLookup.mm:29 > > +#include "Document.h" > > all of these should be 'import' > Fixed! > > Source/WebCore/editing/DictionaryLookup.mm:77 > > +PassRefPtr<Range> rangeExpandedAroundPositionByCharacters(const VisiblePosition& position, int numberOfCharactersToExpand) > > Remind me to make use of this in Editor, where this code was originally > stolen from. > Will do! I made a note of that in the bug I filed! > > Source/WebCore/editing/DictionaryLookup.mm:116 > > + wkExtractWordDefinitionTokenRangeFromContextualString(fullPlainTextString, rangeToPass, options); > > We should probably ditch the WKSI wrappers and add some LookUp SPI headers. > Agreed. Will do this in the future. > > Source/WebKit/mac/WebView/WebActionMenuController.mm:297 > > +static DictionaryPopupInfo performDictionaryLookupForRange(Frame* frame, Range& range, NSDictionary *options) > > Can we put more of this in the WebCore? We talked about this in person. The answer is yes, but not trivially. There are small differences in the code for WK1, unfortunately.
http://trac.webkit.org/changeset/175814