Bug 138766 - Move more CSS properties to the new StyleBuilder
Summary: Move more CSS properties to the new StyleBuilder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 137910
Blocks: 138882 138909 138938 138983 138999
  Show dependency treegraph
 
Reported: 2014-11-15 16:06 PST by Chris Dumez
Modified: 2014-11-21 21:28 PST (History)
3 users (show)

See Also:


Attachments
Patch (24.99 KB, patch)
2014-11-15 16:09 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.79 KB, patch)
2014-11-16 21:02 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.42 KB, patch)
2014-11-17 08:05 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-11-15 16:06:47 PST
Move more CSS properties from DeprecatedStyleBuilder to the new StyleBuilder:
line-height
word-spacing
-webkit-marquee-repetition
-webkit-text-underline-position
Comment 1 Chris Dumez 2014-11-15 16:09:18 PST
Created attachment 241665 [details]
Patch
Comment 2 Darin Adler 2014-11-16 20:11:40 PST
Comment on attachment 241665 [details]
Patch

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

> Source/WebCore/css/StyleBuilderConverter.h:438
> +    unsigned t = 0;

How about combinedPosition instead of t as a name for this variable?

> Source/WebCore/css/StyleBuilderConverter.h:440
> +        TextUnderlinePosition t2 = downcast<CSSPrimitiveValue>(currentValue.get());

How about position instead of t2 as a name for this variable?

> Source/WebCore/css/StyleBuilderCustom.h:497
> +static inline CSSToLengthConversionData csstoLengthConversionDataWithTextZoomFactor(StyleResolver& styleResolver)

Should not be marked "static".

> Source/WebCore/css/StyleBuilderCustom.h:505
> +static inline bool convertLineHeight(StyleResolver& styleResolver, const CSSValue& value, Length& length, float multiplier = 1.f)

Should not be marked "static".

> Source/WebCore/css/StyleBuilderCustom.h:548
> +#if ENABLE(IOS_TEXT_AUTOSIZING)
> +inline void applyInheritLineHeight(StyleResolver& styleResolver)

Should add a blank line here.

> Source/WebCore/css/StyleBuilderCustom.h:580
> +}
> +#endif

Should add a blank line here.
Comment 3 Chris Dumez 2014-11-16 21:02:18 PST
Created attachment 241690 [details]
Patch
Comment 4 WebKit Commit Bot 2014-11-16 21:46:17 PST
Comment on attachment 241690 [details]
Patch

Rejecting attachment 241690 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
bCore/rendering/style/StyleGridItemData.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/StyleGridItemData.o

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/StyleBuilder.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/StyleBuilder.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.appspot.com/results/6263404385796096
Comment 5 Chris Dumez 2014-11-17 08:05:59 PST
Created attachment 241714 [details]
Patch
Comment 6 WebKit Commit Bot 2014-11-17 08:43:53 PST
Comment on attachment 241714 [details]
Patch

Clearing flags on attachment: 241714

Committed r176202: <http://trac.webkit.org/changeset/176202>
Comment 7 WebKit Commit Bot 2014-11-17 08:43:59 PST
All reviewed patches have been landed.  Closing bug.