Bug 188828

Summary: Spelling dots do not scale with page on iOS; share spelling dot painting code between Mac and iOS
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: mmaxfield, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 11   
See Also: https://bugs.webkit.org/show_bug.cgi?id=188762
https://bugs.webkit.org/show_bug.cgi?id=188861
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
For landing none

Daniel Bates
Reported 2018-08-21 16:41:00 PDT
The spelling dots on Mac and iOS have the same visual appearance up to color. As a step towards having the spelling dots in WebKit on iOS more closely match the spelling dots on iOS we should share the same painting code used in WebKit on Mac. A side benefit of sharing more code between Mac and iOS is that this will fix rendering artifacts when painting spelling dots on iOS when the page is zoomed.
Attachments
Patch (55.05 KB, patch)
2018-08-21 16:59 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (44.67 KB, patch)
2018-08-21 17:03 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (50.15 KB, patch)
2018-08-21 18:00 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch (50.27 KB, patch)
2018-08-22 09:40 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
For landing (50.40 KB, patch)
2018-08-23 16:47 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2018-08-21 16:41:42 PDT
Another benefit is that we get to remove a lot of code :P
Daniel Bates
Comment 2 2018-08-21 16:59:04 PDT
Created attachment 347731 [details] Patch
Daniel Bates
Comment 3 2018-08-21 17:01:29 PDT
An example of a rendering artifact when painting the bitmap dots on iOS can be see in attachment #347552 [details] (bug #188762).
Daniel Bates
Comment 4 2018-08-21 17:03:40 PDT
Created attachment 347733 [details] Patch
Simon Fraser (smfr)
Comment 5 2018-08-21 17:04:03 PDT
Comment on attachment 347731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347731&action=review > Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:220 > + // Cocoa platform use the theme to paint the platform document markers. Uses the theme?
Daniel Bates
Comment 6 2018-08-21 17:06:10 PDT
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 347731 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=347731&action=review > > > Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:220 > > + // Cocoa platform use the theme to paint the platform document markers. > > Uses the theme? Maybe "Cocoa platforms use RenderTheme::drawLineForDocumentMarker() to paint the platform document markers."?
Daniel Bates
Comment 7 2018-08-21 18:00:45 PDT
Created attachment 347746 [details] Patch Updated code comment per comment 6. Removed more codez.
Daniel Bates
Comment 8 2018-08-22 09:32:25 PDT
(In reply to Daniel Bates from comment #0) > A side benefit of sharing more code between Mac and iOS is that this will > fix rendering artifacts when painting spelling dots on iOS when the page is > zoomed. This issue is tracked in <rdar://problem/15966403>.
Daniel Bates
Comment 9 2018-08-22 09:40:22 PDT
Created attachment 347809 [details] Patch
Simon Fraser (smfr)
Comment 10 2018-08-23 15:25:40 PDT
Comment on attachment 347809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347809&action=review > Source/WebCore/rendering/RenderThemeCocoa.mm:29 > #if ENABLE(APPLE_PAY) Er, why are all these #includes under ENABLE(APPLE_PAY)? I think you'll have to move out those you need for drawLineForDocumentMarker().
Daniel Bates
Comment 11 2018-08-23 16:20:03 PDT
Comment on attachment 347809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347809&action=review >> Source/WebCore/rendering/RenderThemeCocoa.mm:29 >> #if ENABLE(APPLE_PAY) > > Er, why are all these #includes under ENABLE(APPLE_PAY)? I think you'll have to move out those you need for drawLineForDocumentMarker(). Will fix before landing.
Daniel Bates
Comment 12 2018-08-23 16:47:24 PDT
Created attachment 347972 [details] For landing
Daniel Bates
Comment 13 2018-08-27 10:06:22 PDT
Committed r235378: <https://trac.webkit.org/changeset/235378>
Radar WebKit Bug Importer
Comment 14 2018-08-27 10:07:26 PDT
<rdar://problem/43758062>
Daniel Bates
Comment 15 2020-01-16 10:07:45 PST
*** Bug 135666 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.