Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(58)

Issue 2641083002: Implements CSSPropertyAPI for letter and word spacing (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 1 week ago by gracie
Modified:
5 months, 1 week ago
Reviewers:
sashab, aazzam
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implements CSSPropertyAPI for letter and word spacing A part of Project Ribbon, separating the parsing logic for CSS properties from the parser into an API. This patch removes CSSPropertyLetterSpacing, and CSSPropertyWordSpacing from the switch statement in parseSingleValue, and calls the API instead. A function pointer to the parseSingleValue function from the API for the letter and word spacing properties is stored in a CSSPropertyDescriptor, and is called from CSSPropertyParser. This patch: - Adds CSSPropertyAPILetterAndWordSpacing.cpp to the BUILD.gn file. - Adds api_class=CSSPropertyAPILetterAndWordSpacing flag to each border class property in CSSProperties.in, which indicates that CSSPropertyAPILetterAndWordSpacing.h is generated. - Moves the parsing logic for letter and word spacing from CSSPropertyParser.cpp to CSSPropertyAPILetterAndWordSpacing.cpp, which implements CSSPropertyAPI.h. BUG=668012 Review-Url: https://codereview.chromium.org/2641083002 Cr-Commit-Position: refs/heads/master@{#444675} Committed: https://chromium.googlesource.com/chromium/src/+/0006a31fd4a9fcf1b39a099f9a3056856fa8e063

Patch Set 1 #

Total comments: 1

Patch Set 2 : rebase update #

Messages

Total messages: 20 (11 generated)
gracie
5 months, 1 week ago (2017-01-19 00:35:00 UTC) #4
aazzam
thanks grace!! lgtm, ptal sasha :)
5 months, 1 week ago (2017-01-19 00:35:44 UTC) #6
sashab
LGTM pending anna's approval on the naming convention :) https://codereview.chromium.org/2641083002/diff/1/third_party/WebKit/Source/core/css/properties/CSSPropertyAPILetterAndWordSpacing.cpp File third_party/WebKit/Source/core/css/properties/CSSPropertyAPILetterAndWordSpacing.cpp (right): https://codereview.chromium.org/2641083002/diff/1/third_party/WebKit/Source/core/css/properties/CSSPropertyAPILetterAndWordSpacing.cpp#newcode12 third_party/WebKit/Source/core/css/properties/CSSPropertyAPILetterAndWordSpacing.cpp:12: ...
5 months, 1 week ago (2017-01-19 00:47:24 UTC) #7
sashab
Oh sorry also please wrap CL description to 72 chars :) thanks
5 months, 1 week ago (2017-01-19 00:47:37 UTC) #8
gracie
On 2017/01/19 00:47:37, sashab wrote: > Oh sorry also please wrap CL description to 72 ...
5 months, 1 week ago (2017-01-19 02:50:13 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2641083002/1
5 months, 1 week ago (2017-01-19 02:51:01 UTC) #12
commit-bot: I haz the power
Failed to apply patch for third_party/WebKit/Source/core/css/BUILD.gn: While running git apply --index -p1; error: patch failed: ...
5 months, 1 week ago (2017-01-19 04:49:01 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2641083002/20001
5 months, 1 week ago (2017-01-19 05:49:39 UTC) #17
commit-bot: I haz the power
5 months, 1 week ago (2017-01-19 07:41:26 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0006a31fd4a9fcf1b39a099f9a30...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318