Bug 162490 – [Cocoa] ☔ Implement font-variation-settings

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[Cocoa] Implement font-variation-settings

[Myles C. Maxfield]

Created attachment 289681 [details]
WIP

[Myles C. Maxfield]

Created attachment 289742 [details]
Needs animation support

[Myles C. Maxfield]

Created attachment 289803 [details]
Needs Tests

[Myles C. Maxfield]

Created attachment 290068 [details]
WIP

[WebKit Commit Bot]

Attachment 290068 [details] did not pass style-queue:


ERROR: LayoutTests/TestExpectations:992:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/TestExpectations:993:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2670:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2671:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/platform/mac/TestExpectations:1461:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/platform/mac/TestExpectations:1462:  Path does not exist.  [test/expectations] [5]
Total errors found: 6 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 290068 [details]
WIP

Attachment 290068 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2160260

New failing tests:
fast/text/trak-optimizeLegibility.html
fast/text/variations/exist.html

[Build Bot]

Created attachment 290069 [details]
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

Created attachment 290151 [details]
Patch

[Myles C. Maxfield]

Created attachment 290152 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 290151 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290151&action=review

I think we need to have a runtime flag for the new font variation stuff. We can't expose font-variation-settings without it.

> Source/WebCore/page/animation/CSSPropertyAnimation.cpp:557
> +        // If the style pointers are the same, don't bother doing the test.
> +        // If either is null, return false. If both are null, return true.
> +        if (a == b)
> +            return true;
> +        if (!a || !b)
> +            return false;
> +
> +        const FontVariationSettings& variationSettingsA = (a->*m_getter)();
> +        const FontVariationSettings& variationSettingsB = (b->*m_getter)();
> +        return variationSettingsA == variationSettingsB;

See arePointingToEqualData()

> Source/WebCore/platform/graphics/FontTaggedSettings.cpp:2
> + * Copyright (C) 2011 Google Inc. All rights reserved.

Need to add an Apple line here if anything changed.

[Myles C. Maxfield]

(In reply to comment #10)
> > Source/WebCore/page/animation/CSSPropertyAnimation.cpp:557
> > +        // If the style pointers are the same, don't bother doing the test.
> > +        // If either is null, return false. If both are null, return true.
> > +        if (a == b)
> > +            return true;
> > +        if (!a || !b)
> > +            return false;
> > +
> > +        const FontVariationSettings& variationSettingsA = (a->*m_getter)();
> > +        const FontVariationSettings& variationSettingsB = (b->*m_getter)();
> > +        return variationSettingsA == variationSettingsB;
> 
> See arePointingToEqualData()
> 

That function seems to be for pointers, but these variables are references.

[Myles C. Maxfield]

(In reply to comment #10)
> Comment on attachment 290151 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290151&action=review
> 
> I think we need to have a runtime flag for the new font variation stuff. We
> can't expose font-variation-settings without it.
> 

I'll ask Sam about this tomorrow.

[Myles C. Maxfield]

Created attachment 290160 [details]
Patch

[Myles C. Maxfield]

Created attachment 290239 [details]
Patch

[Build Bot]

Comment on attachment 290239 [details]
Patch

Attachment 290239 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2170567

New failing tests:
fast/text/trak-optimizeLegibility.html

[Build Bot]

Created attachment 290251 [details]
Archive of layout-test-results from ews124 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

Created attachment 290261 [details]
Patch

[Myles C. Maxfield]

Created attachment 290262 [details]
Patch

[Myles C. Maxfield]

Created attachment 290277 [details]
Patch

[Build Bot]

Comment on attachment 290277 [details]
Patch

Attachment 290277 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2172017

New failing tests:
fast/images/percent-height-image.html

[Build Bot]

Created attachment 290286 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5