Bug 32770 – [GTK] Spell check works on selection but it should work on unselected words

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Christian Dywan]

The way spell check works is this: you enable it, type a misspelled word, a red underline appears. If you right-click the word, the underline vanishes and you see edit items. If you select the word, and then right-click you see spelling suggestions and edit items.

For comparison, Abiword: type a misspelled word, a red underline appears. If you right-click the word, you see spelling suggestions. If you select the word, then right-click, you see an editing menu.

I think what WebKitGTK+ is basically the opposite of what it should do, which is what Abiword does.

Incidentally the Mac always selects the word below the mouse pointer when you right-click, that is why the Mac port has no problem with the current logic.

[Christian Dywan]

Created attachment 45239 [details]
Pass a string to Editor and make GTK+ not use the selected text but the text in the node.

The patch allows the behaviour as described above, and the GTK+ port makes use of it, while other ports behave as before. Note that I didn't fully port Mac code paths in Editor to support the non-selection behaviour since they are not used and I can't really test it. If you think it's useful anyhow and agree with the approach, I can try to make the Mac changes.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45239 [details] without any errors.

[WebKit Review Bot]

Attachment 45239 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/133935

[Christian Dywan]

Created attachment 45240 [details]
Pass a string to Editor and make GTK+ not use the selected text but the text in the node. #2

Updated. I accidentally put a bracket outside of an #if.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45240 [details] without any errors.

[Eric Seidel (no email)]

I feel like Chromium had to do a similar behavoir change, so I'm surprised this code change is this large.

[Christian Dywan]

(In reply to comment #6)
> I feel like Chromium had to do a similar behavoir change, so I'm surprised this
> code change is this large.

The reason is that Chormium doesn't use the common code path. ContextMenuClientImpl in WebKit/chromium does its own handling.

[Evan Martin]

I think this needs a test.

[Eric Seidel (no email)]

Whoever did the Chromium work should probably be CC'd on this.  If I remember correctly there was recently someone from Apple who was re-writing the spell check code too.

[Adam Barth]

Attachment 45240 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/152834

[Eric Seidel (no email)]

Ah yes.  Doug was re-writing spell-check from Apple's side of things.  I'm not sure he'd actually be interested in this bug, but I'll add him to the CC list just in case.  (He can always remove himself.)

[Eric Seidel (no email)]

Comment on attachment 45240 [details]
Pass a string to Editor and make GTK+ not use the selected text but the text in the node. #2

This would break the Mac build:
/Users/abarth/git/webkit-queue/WebCore/editing/Editor.cpp: In member function 'WTF::Vector<WebCore::String, 0ul> WebCore::Editor::guessesForMisspelledOrUngrammaticalSelection(bool&, bool&, const WebCore::String&, const WebCore::HitTestResult&)':
/Users/abarth/git/webkit-queue/WebCore/editing/Editor.cpp:2173: error: cannot convert 'const WebCore::String' to 'WebCore::Range*' for argument '2' to 'WTF::Vector<WebCore::String, 0ul> WebCore::guessesForMisspelledOrUngrammaticalRange(WebCore::EditorClient*, WebCore::Range*, bool, bool&, bool&)'
cc1plus: warnings being treated as errors
/Users/abarth/git/webkit-queue/WebCore/editing/Editor.cpp: At global scope:
/Users/abarth/git/webkit-queue/WebCore/editing/Editor.cpp:2170: warning: unused parameter 'result'

[Michael Catanzaro]

This was fixed, somehow, a long time ago.