<rdar://problem/35896516>
Created attachment 330934 [details] Fixes the bug
Comment on attachment 330934 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=330934&action=review > Source/WebCore/rendering/InlineTextBox.cpp:1047 > + paintTextSubrangeBackground(context, boxOrigin, Color::compositionFill, clampedOffset(renderer().frame().editor().compositionStart()), clampedOffset(renderer().frame().editor().compositionEnd())); We should make Color(RGBA32 color, bool valid = true) explicit since RGBA32 is just unsigned so that mistakes like this will be harder to make (not suggesting that we should do it in this patch or that you should do it).
Comment on attachment 330934 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=330934&action=review Thanks for the review! > Source/WebCore/ChangeLog:11 > + Test: editing/composition-highlight.html Whoops, test here should be "editing/marked-text-appearance.html", not "editing/composition-highlight.html". I'll fix this before landing. >> Source/WebCore/rendering/InlineTextBox.cpp:1047 >> + paintTextSubrangeBackground(context, boxOrigin, Color::compositionFill, clampedOffset(renderer().frame().editor().compositionStart()), clampedOffset(renderer().frame().editor().compositionEnd())); > > We should make Color(RGBA32 color, bool valid = true) explicit since RGBA32 is just unsigned so that mistakes like this will be harder to make > (not suggesting that we should do it in this patch or that you should do it). Definitely a good idea. I think Dan Bates had ideas on how to make this more robust, too...I'll file a followup for this.
Comment on attachment 330934 [details] Fixes the bug Attachment 330934 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6025473 New failing tests: webgl/1.0.2/conformance/uniforms/uniform-default-values.html
Created attachment 330971 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 330934 [details] Fixes the bug Attachment 330934 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6026655 Number of test failures exceeded the failure limit.
Created attachment 330987 [details] Archive of layout-test-results from ews105 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 330994 [details] Patch for landing
Comment on attachment 330994 [details] Patch for landing Clearing flags on attachment: 330994 Committed r226753: <https://trac.webkit.org/changeset/226753>
Comment on attachment 330934 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=330934&action=review >>> Source/WebCore/rendering/InlineTextBox.cpp:1047 >>> + paintTextSubrangeBackground(context, boxOrigin, Color::compositionFill, clampedOffset(renderer().frame().editor().compositionStart()), clampedOffset(renderer().frame().editor().compositionEnd())); >> >> We should make Color(RGBA32 color, bool valid = true) explicit since RGBA32 is just unsigned so that mistakes like this will be harder to make >> (not suggesting that we should do it in this patch or that you should do it). > > Definitely a good idea. I think Dan Bates had ideas on how to make this more robust, too...I'll file a followup for this. I have a solution to this ambiguity coming in my color patch; the total patch is in bug 180620 but I will be breaking off smaller pieces of it and landing them.