RESOLVED FIXED 122365
Include misspelling dot gap width when centering misspelling dots 
https://bugs.webkit.org/show_bug.cgi?id=122365
Summary Include misspelling dot gap width when centering misspelling dots
Myles C. Maxfield
Reported 2013-10-04 17:33:48 PDT
Include misspelling dot gap width when centering misspelling dots
Attachments
Patch (93.68 KB, patch)
2013-10-22 14:18 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (23.06 KB, patch)
2013-10-22 16:05 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (23.89 KB, patch)
2013-10-23 13:10 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (44.49 KB, patch)
2013-10-23 15:51 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2013-10-22 14:18:06 PDT
Created attachment 214881 [details] Patch
Myles C. Maxfield
Comment 2 2013-10-22 16:05:52 PDT
Created attachment 214897 [details] Patch
Radar WebKit Bug Importer
Comment 3 2013-10-23 10:43:42 PDT
<rdar://problem/15299813>
Jon Lee
Comment 4 2013-10-23 11:34:02 PDT
Comment on attachment 214897 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214897&action=review > Source/WebCore/ChangeLog:11 > + (WebCore::GraphicsContext::drawLineForDocumentMarker): More explanation here please. Why is there a new (widthMod > 0) check? What is the new code doing to the offset? Why is it a floor()? > LayoutTests/editing/spelling/centering-misspelling-dots-expected.txt:58 > +caret: position 14 of child 0 {#text} of child 3 {DIV} of body Is having expected text useful for this test?
Myles C. Maxfield
Comment 5 2013-10-23 13:08:14 PDT
Comment on attachment 214897 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214897&action=review >> Source/WebCore/ChangeLog:11 >> + (WebCore::GraphicsContext::drawLineForDocumentMarker): > > More explanation here please. Why is there a new (widthMod > 0) check? What is the new code doing to the offset? Why is it a floor()? Done. >> LayoutTests/editing/spelling/centering-misspelling-dots-expected.txt:58 >> +caret: position 14 of child 0 {#text} of child 3 {DIV} of body > > Is having expected text useful for this test? I don't think it's possible to run a pixel test without expected text. When I try, it says that the test failed due to missing results.
Myles C. Maxfield
Comment 6 2013-10-23 13:10:10 PDT
Created attachment 214985 [details] Patch
Jon Lee
Comment 7 2013-10-23 15:08:21 PDT
Comment on attachment 214985 [details] Patch Provisional r=me.
Simon Fraser (smfr)
Comment 8 2013-10-23 15:09:56 PDT
Comment on attachment 214985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214985&action=review > Source/WebCore/platform/graphics/mac/GraphicsContextMac.mm:-170 > - // NOTE: Code here used to shift the underline to the left and increase the width > - // to make sure everything gets underlined, but that results in drawing out of > - // bounds (e.g. when at the edge of a view) and could make it appear that the > - // space between adjacent misspelled words was underlined. I would like to see your testcase testing "the space between misspelled words".
Myles C. Maxfield
Comment 9 2013-10-23 15:51:29 PDT
Created attachment 215004 [details] Patch
WebKit Commit Bot
Comment 10 2013-10-23 16:49:20 PDT
Comment on attachment 215004 [details] Patch Clearing flags on attachment: 215004 Committed r157900: <http://trac.webkit.org/changeset/157900>
WebKit Commit Bot
Comment 11 2013-10-23 16:49:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.