It is currently possible to apply a mix of the css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color. This is unfortunate, we should either use the combination stroke-width/stroke-color or the combination -webkit-text-stroke-width/-webkit-text-stroke-color. Example: <div style="stroke-width: 1px">Text</div> Since stroke-color is not specified here, WebKit will currently fall back to the -webkit-text-stroke-color value, and not use the default transparent value of the stroke-color property. The above text is currently rendered with stroke, this is incorrect.
<rdar://problem/33466294>
Created attachment 316318 [details] Patch
Comment on attachment 316318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316318&action=review > LayoutTests/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=174737 <rdar://problem/33466294>
I think Dean or Simon should review this. It seems like a reasonable change, and it's a shame it hasn't been landed.
Comment on attachment 316318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316318&action=review Please consult Antti and Hyatt. > Source/WebCore/ChangeLog:3 > + The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed. CSS (uppercase) > Source/WebCore/ChangeLog:12 > + Previously, the stroke width and stroke color would independently fall back to the -webkit-text-stroke-width > + and -webkit-text-stroke-color values, if stroke-width and/or stroke-color were not explicitly specified. The > + new strategy is to fall back to the -webkit-text-stroke-width and -webkit-text-stroke-color value combination, > + if the stroke-width and stroke-color combination does not result in a visible stroke. The fallback is problematic because of the differing initial values for stroke-color and -webkit-stroke-color, right (transparent vs. currentColor?). I wonder if it would be simpler to just alias these properties, and change the default -webkit-text-stroke-color. We'd have to see if that breaks content that we care about.
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 316318 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316318&action=review > > Please consult Antti and Hyatt. > > > Source/WebCore/ChangeLog:3 > > + The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed. > > CSS (uppercase) > > > Source/WebCore/ChangeLog:12 > > + Previously, the stroke width and stroke color would independently fall back to the -webkit-text-stroke-width > > + and -webkit-text-stroke-color values, if stroke-width and/or stroke-color were not explicitly specified. The > > + new strategy is to fall back to the -webkit-text-stroke-width and -webkit-text-stroke-color value combination, > > + if the stroke-width and stroke-color combination does not result in a visible stroke. > > The fallback is problematic because of the differing initial values for > stroke-color and -webkit-stroke-color, right (transparent vs. > currentColor?). I wonder if it would be simpler to just alias these > properties, and change the default -webkit-text-stroke-color. We'd have to > see if that breaks content that we care about. That's a good point, I will look into it :) Thanks for reviewing, all!
Created attachment 321900 [details] Patch
Comment on attachment 321900 [details] Patch Attachment 321900 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4670961 Number of test failures exceeded the failure limit.
Created attachment 321912 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 321900 [details] Patch Attachment 321900 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4670970 Number of test failures exceeded the failure limit.
Created attachment 321914 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 321900 [details] Patch Attachment 321900 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4671154 Number of test failures exceeded the failure limit.
Created attachment 321917 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 321900 [details] Patch Attachment 321900 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4671205 New failing tests: tables/mozilla/bugs/bug46623-1.html fast/css/remove-shorthand.html fast/selectors/017.html fast/css/stroke-miterlimit-large.html fast/css/getPropertyValue-webkit-text-stroke.html fast/css/child-style-can-override-visited-style.html css2.1/t051103-c21-hover-ln-00-e-i.html css2.1/t060403-c21-pseu-cls-00-e-i.html css3/selectors3/xhtml/css3-modsel-17.xml fast/css/style-tag-display-none.html svg/css/text-shadow-multiple.xhtml css2.1/t0511-c21-pseud-link-03-e.html css2.1/t051103-c21-activ-ln-00-e-i.html css2.1/t051103-c21-focus-ln-00-e-i.html css3/selectors3/xml/css3-modsel-17.xml css2.1/t060403-c21-pseu-id-00-e-i.html fast/css/shadow-multiple.html fast/selectors/visited-descendant.html tables/mozilla/bugs/bug8950.html css3/selectors3/html/css3-modsel-17.html fast/dom/34176.html fast/selectors/061.html media/track/track-css-stroke-cues.html fast/css/stroke-miterlimit-zero.html tables/mozilla/bugs/bug7342.html tables/mozilla/bugs/bug51140.html css2.1/t0511-c21-pseud-link-02-e.html
Created attachment 321923 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 316318 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316318&action=review > > Please consult Antti and Hyatt. > > > Source/WebCore/ChangeLog:3 > > + The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed. > > CSS (uppercase) > > > Source/WebCore/ChangeLog:12 > > + Previously, the stroke width and stroke color would independently fall back to the -webkit-text-stroke-width > > + and -webkit-text-stroke-color values, if stroke-width and/or stroke-color were not explicitly specified. The > > + new strategy is to fall back to the -webkit-text-stroke-width and -webkit-text-stroke-color value combination, > > + if the stroke-width and stroke-color combination does not result in a visible stroke. > > The fallback is problematic because of the differing initial values for > stroke-color and -webkit-stroke-color, right (transparent vs. > currentColor?). I wonder if it would be simpler to just alias these > properties, and change the default -webkit-text-stroke-color. We'd have to > see if that breaks content that we care about. After having looked closer into this, it seems aliasing will break setting stroke properties like '-webkit-text-stroke-width: thin;', and '-webkit-text-stroke: thin orange;'.
Created attachment 323200 [details] Patch
Attachment 323200 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 323246 [details] Patch
(In reply to Per Arne Vollan from comment #19) > Created attachment 323246 [details] > Patch Since there is a minor risk of breaking existing content with the aliasing approach, I have suggested a different fix to the bug in the last patch.
Simon, Antti, Hyatt?
Comment on attachment 323246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323246&action=review Seems reasonable, r=me > Source/WebCore/rendering/style/RenderStyle.cpp:2358 > - if (!hasExplicitlySetStrokeWidth()) > + if (!hasExplicitlySetStrokeColor()) > return textStrokeWidth(); This needs a comment. It really reads like a bug.
(In reply to Antti Koivisto from comment #22) > Comment on attachment 323246 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=323246&action=review > > Seems reasonable, r=me > > > Source/WebCore/rendering/style/RenderStyle.cpp:2358 > > - if (!hasExplicitlySetStrokeWidth()) > > + if (!hasExplicitlySetStrokeColor()) > > return textStrokeWidth(); > > This needs a comment. It really reads like a bug. Thanks for reviewing! I will add a comment before landing.
Committed r224780: <https://trac.webkit.org/changeset/224780/webkit>.