WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
138208
Move -webkit-marquee-increment property to the new StyleBuilder
https://bugs.webkit.org/show_bug.cgi?id=138208
Summary
Move -webkit-marquee-increment property to the new StyleBuilder
Chris Dumez
Reported
2014-10-29 21:20:52 PDT
Move -webkit-marquee-increment property from DeprecatedStyleBuilder to the new StyleBuilder so that it is generated from CSSPropertyNames.in.
Attachments
Patch
(10.65 KB, patch)
2014-10-29 21:52 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(12.50 KB, patch)
2014-10-30 10:29 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(12.38 KB, patch)
2014-10-30 14:37 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(12.38 KB, patch)
2014-10-30 20:42 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2014-10-29 21:52:53 PDT
Created
attachment 240657
[details]
Patch
Sam Weinig
Comment 2
2014-10-30 06:08:46 PDT
Comment on
attachment 240657
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=240657&action=review
> Source/WebCore/css/StyleBuilderCustom.h:47 > + auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
What guarantees this cast is safe? Could we have applyValueWebkitMarqueeIncrement() take a CSSPrimitiveValue instead of a CSSValue?
Chris Dumez
Comment 3
2014-10-30 10:29:22 PDT
Created
attachment 240680
[details]
Patch
Chris Dumez
Comment 4
2014-10-30 10:33:46 PDT
Comment on
attachment 240657
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=240657&action=review
>> Source/WebCore/css/StyleBuilderCustom.h:47 >> + auto& primitiveValue = downcast<CSSPrimitiveValue>(value); > > What guarantees this cast is safe? Could we have applyValueWebkitMarqueeIncrement() take a CSSPrimitiveValue instead of a CSSValue?
The CSS Parser is already marking sure the value for -webkit-marquee-increment property is a CSSPrimitiveValue so this cast should be safe and the assertion in downcast<>() should protect us against a mismatch between the CSS Parser and the StyleBuilder. The reason the StyleBuilder functions take a CSSValue and not a CSSPrimitiveValue is because not all properties have a CSSPrimitiveValue as value. Some have a CSSValueList for e.g. and make prop.pl does not have any way to know as far as I can tell because this information is coded in the CSSParser, not CSSPropertyNames.in. I have added a new layout test in the latest patch iteration to gain coverage for non CSSPrimitive values on -webkit-marquee-increment, and make sure we don't crash.
Chris Dumez
Comment 5
2014-10-30 14:37:13 PDT
Created
attachment 240702
[details]
Patch
Chris Dumez
Comment 6
2014-10-30 20:42:42 PDT
Created
attachment 240723
[details]
Patch
Chris Dumez
Comment 7
2014-10-31 09:58:09 PDT
ping review?
Andreas Kling
Comment 8
2014-10-31 16:19:26 PDT
Comment on
attachment 240723
[details]
Patch r=me
WebKit Commit Bot
Comment 9
2014-10-31 16:46:15 PDT
Comment on
attachment 240723
[details]
Patch Clearing flags on attachment: 240723 Committed
r175439
: <
http://trac.webkit.org/changeset/175439
>
WebKit Commit Bot
Comment 10
2014-10-31 16:46:20 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug