Bug 41323

Summary: [WinCairo] drawLineForMisspellingOrBadGrammar is Not Functional
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mrobinson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Patch enabling 'squiggle' for bad spelling/grammar.
none
Updated patch, isolating the squiggle code in its own LGPL-licensed file.
none
Silence some style warnings.
none
Style cleanups.
none
More style cleanups.
none
Good Grief! Another attempt... gustavo: review+

Description Brent Fulgham 2010-06-28 18:10:28 PDT
The implementation of drawLineForMisspellingOrBadGrammar is not functional for the WinCairo build.
Comment 1 Brent Fulgham 2010-06-29 09:53:15 PDT
Created attachment 60030 [details]
Patch enabling 'squiggle' for bad spelling/grammar.

Patch implements drawing logic for creating a squiggly line beneath misspelled or ungrammatical content during editing.  Logic is based on the Pango sources, so that the WinCairo port matches behavior of the other Cairo-based ports.
Comment 2 Martin Robinson 2010-06-29 10:54:43 PDT
Great patch! The only change I would suggest is either changing the code to align with WebKit style (HEIGHT_SQUARES) or isolating it in a file that can be exempted from WebKit style checks.
Comment 3 Brent Fulgham 2010-06-29 23:29:44 PDT
Created attachment 60088 [details]
Updated patch, isolating the squiggle code in its own LGPL-licensed file.

Update based on everyone's comments up to this point.
Comment 4 WebKit Review Bot 2010-06-29 23:35:22 PDT
Attachment 60088 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:24:  Found other header before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:70:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:74:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:97:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Brent Fulgham 2010-06-30 16:02:05 PDT
Created attachment 60161 [details]
Silence some style warnings.

I can't resolve the one-line if/else complaint, because the no-op (comment) line requires braces to compile.  I could switch to a C-style comment to resolve, but this seems like a reasonable way to handle the stanza in question.
Comment 6 WebKit Review Bot 2010-06-30 16:54:20 PDT
Attachment 60161 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.cpp:78:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Brent Fulgham 2010-07-01 10:18:30 PDT
Created attachment 60260 [details]
Style cleanups.
Comment 8 WebKit Review Bot 2010-07-01 10:19:25 PDT
Attachment 60260 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:24:  Header file should not contain WebCore config.h. Should be: alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:77:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Brent Fulgham 2010-07-01 12:45:20 PDT
Created attachment 60274 [details]
More style cleanups.

Revised per style check guidelines.
Comment 10 WebKit Review Bot 2010-07-01 12:46:10 PDT
Attachment 60274 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:74:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:75:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:76:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/cairo/DrawErrorUnderline.h:77:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Brent Fulgham 2010-07-01 13:12:19 PDT
Created attachment 60278 [details]
Good Grief! Another attempt...
Comment 12 Gustavo Noronha (kov) 2010-07-02 09:47:55 PDT
Comment on attachment 60278 [details]
Good Grief! Another attempt...

OK, looks sane
Comment 13 Brent Fulgham 2010-07-02 12:27:27 PDT
Landed in http://trac.webkit.org/changeset/62388.