Bug 161986 – Add CSS -webkit-appearance property for Apple Pay buttons

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

[Anders Carlsson]

Add CSS -webkit-appearance property for Apple Pay buttons

[Anders Carlsson]

Created attachment 288859 [details]
Patch

[Anders Carlsson]

Created attachment 288861 [details]
Patch

[Anders Carlsson]

Created attachment 288863 [details]
Patch

[Anders Carlsson]

Created attachment 288870 [details]
Patch

[Anders Carlsson]

Created attachment 288879 [details]
Patch

[Anders Carlsson]

Created attachment 288886 [details]
Patch

[Dean Jackson]

Comment on attachment 288886 [details]
Patch

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

If it is possible to put the PDF artwork in Open Source, it would be good to have some ref tests for the combination of 3 styles * 4 types.

> Source/WebCore/css/CSSPrimitiveValueMappings.h:5499
> +    default:
> +        ASSERT_NOT_REACHED();
> +        break;
> +    }

Maybe pick one style between this ^^^^

> Source/WebCore/css/CSSPrimitiveValueMappings.h:5515
> +    default:
> +        break;
> +    }
> +    ASSERT_NOT_REACHED();

... and this ^^^^?

Although maybe this is standard.

Same with the ApplePayButtonType parts.

> Source/WebCore/css/CSSPropertyNames.in:687
> +-apple-pay-button-style [NameForMethods=ApplePayButtonStyle]
> +-apple-pay-button-type [NameForMethods=ApplePayButtonType]

Wouldn't you get these names by default?

> Source/WebCore/rendering/style/RenderStyle.h:1746
> +#if ENABLE(APPLE_PAY)
> +    void setApplePayButtonStyle(ApplePayButtonStyle v) { SET_VAR(rareInheritedData, applePayButtonStyle, static_cast<unsigned>(v)); }
> +    void setApplePayButtonType(ApplePayButtonType v) { SET_VAR(rareInheritedData, applePayButtonType, static_cast<unsigned>(v)); }
> +#endif

I think these should be in rareNonInheritedData, unless you think you'd want to set the style on a parent element and have it inherit down.

[Anders Carlsson]

(In reply to comment #7)
> Comment on attachment 288886 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=288886&action=review
> 
> If it is possible to put the PDF artwork in Open Source, it would be good to
> have some ref tests for the combination of 3 styles * 4 types.

I don't think it's possible.

> 
> > Source/WebCore/css/CSSPrimitiveValueMappings.h:5499
> > +    default:
> > +        ASSERT_NOT_REACHED();
> > +        break;
> > +    }
> 
> Maybe pick one style between this ^^^^
> 
> > Source/WebCore/css/CSSPrimitiveValueMappings.h:5515
> > +    default:
> > +        break;
> > +    }
> > +    ASSERT_NOT_REACHED();
> 
> ... and this ^^^^?
> 
> Although maybe this is standard.

Yeah, just matched the existing switch statements.

> 
> Same with the ApplePayButtonType parts.
> 
> > Source/WebCore/css/CSSPropertyNames.in:687
> > +-apple-pay-button-style [NameForMethods=ApplePayButtonStyle]
> > +-apple-pay-button-type [NameForMethods=ApplePayButtonType]
> 
> Wouldn't you get these names by default?

I think so.

> 
> > Source/WebCore/rendering/style/RenderStyle.h:1746
> > +#if ENABLE(APPLE_PAY)
> > +    void setApplePayButtonStyle(ApplePayButtonStyle v) { SET_VAR(rareInheritedData, applePayButtonStyle, static_cast<unsigned>(v)); }
> > +    void setApplePayButtonType(ApplePayButtonType v) { SET_VAR(rareInheritedData, applePayButtonType, static_cast<unsigned>(v)); }
> > +#endif
> 
> I think these should be in rareNonInheritedData, unless you think you'd want
> to set the style on a parent element and have it inherit down.

Yes, you're right.

[Anders Carlsson]

Committed r205980: <http://trac.webkit.org/changeset/205980>

[Simon Fraser (smfr)]

r-, no tests :|