Summary: | [macOS] Fix some font attribute conversion bugs in preparation for "Font > Styles…" support in WebKit2 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | HTML Editing | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bdakin, commit-queue, mitz, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2018-10-04 13:24:53 PDT
...we can address these bugs in parallel. Created attachment 351624 [details]
Patch
Created attachment 351625 [details]
Test tweak
Created attachment 351626 [details]
Add rdar to ChangeLog
Comment on attachment 351626 [details] Add rdar to ChangeLog View in context: https://bugs.webkit.org/attachment.cgi?id=351626&action=review Nice bug fixes! > Source/WebCore/ChangeLog:19 > + Currently, we bail from adding a font shadow if the shadow's offset is empty. However, valid shadow offsets may > + have negative dimensions, so a check for `isZero()` should be used instead. It's very strange / unintuitive that isEmpty() returns true for negative offsets... > Source/WebCore/ChangeLog:26 > + transparent color (this scenario is exercised when using "Font > Stylesâ¦" to specify a font style without a What's up with "â¦"? (In reply to Ryosuke Niwa from comment #7) > Comment on attachment 351626 [details] > Add rdar to ChangeLog > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351626&action=review > > Nice bug fixes! > > > Source/WebCore/ChangeLog:19 > > + Currently, we bail from adding a font shadow if the shadow's offset is empty. However, valid shadow offsets may > > + have negative dimensions, so a check for `isZero()` should be used instead. > > It's very strange / unintuitive that isEmpty() returns true for negative > offsets... Yeah... :/ I think this makes it easier than it should be to make this kind of mistake for FloatSizes that represent offsets, like this. > > > Source/WebCore/ChangeLog:26 > > + transparent color (this scenario is exercised when using "Font > Stylesâ¦" to specify a font style without a > > What's up with "â¦"? This is actually an ellipsis '…' character, but the Bugzilla patch review tool apparently displays this as "â¦". It looks normal outside of the Bugzilla patch review page. Comment on attachment 351626 [details] Add rdar to ChangeLog Clearing flags on attachment: 351626 Committed r236854: <https://trac.webkit.org/changeset/236854> All reviewed patches have been landed. Closing bug. |