Bug 41323 - [WinCairo] drawLineForMisspellingOrBadGrammar is Not Functional
Summary: [WinCairo] drawLineForMisspellingOrBadGrammar is Not Functional
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-28 18:10 PDT by Brent Fulgham
Modified: 2010-07-02 12:27 PDT (History)
2 users (show)

See Also:


Attachments
Patch enabling 'squiggle' for bad spelling/grammar. (5.45 KB, patch)
2010-06-29 09:53 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Updated patch, isolating the squiggle code in its own LGPL-licensed file. (7.49 KB, patch)
2010-06-29 23:29 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Silence some style warnings. (7.21 KB, patch)
2010-06-30 16:02 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Style cleanups. (7.21 KB, patch)
2010-07-01 10:18 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
More style cleanups. (7.07 KB, patch)
2010-07-01 12:45 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Good Grief! Another attempt... (7.10 KB, patch)
2010-07-01 13:12 PDT, Brent Fulgham
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.