Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(556)

Issue 339713008: Treat marker like a real shorthand for initial and inherit (Closed)

Created:
6 years, 5 months ago by alancutter (OOO until 2018)
Modified:
6 years, 1 month ago
Reviewers:
pdr., Timothy Loh
CC:
blink-reviews, darktears
Project:
blink
Visibility:
Public.

Description

Treat marker like a real shorthand for initial and inherit Setting the SVG CSS property "marker" to initial or inherit would hit an ASSERT in StyleBuilder::applyProperty(). Because marker is not handled by shorthandForProperty() despite it being a shorthand property it leaks into the style declaration without expansion due to special case handling of initial and inherit in CSSPropertyParser::parseValue(). Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184427

Patch Set 1 #

Patch Set 2 : Rebased #

Patch Set 3 : Shorthand change #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -2 lines) Patch
A LayoutTests/svg/css/marker-crash.html View 1 chunk +7 lines, -0 lines 0 comments Download
A + LayoutTests/svg/css/marker-crash-expected.txt View 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/build/scripts/templates/StylePropertyShorthand.cpp.tmpl View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/StylePropertyShorthandCustom.cpp View 1 2 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 13 (3 generated)
alancutter (OOO until 2018)
6 years, 5 months ago (2014-06-26 07:34:14 UTC) #1
Timothy Loh
I think this makes the check in isExpandedShorthandForAll for marker unneeded. Actually the check for ...
6 years, 5 months ago (2014-06-26 07:47:10 UTC) #2
alancutter (OOO until 2018)
The CQ bit was checked by alancutter@chromium.org
6 years, 5 months ago (2014-06-27 00:43:32 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alancutter@chromium.org/339713008/1
6 years, 5 months ago (2014-06-27 00:44:18 UTC) #4
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-06-27 00:44:19 UTC) #5
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 5 months ago (2014-06-27 00:44:19 UTC) #6
Timothy Loh
On 2014/06/26 07:34:14, alancutter wrote: lgtm with a rebase and fixing up the earlier comment ...
6 years, 4 months ago (2014-08-15 09:45:14 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/339713008/20001
6 years, 1 month ago (2014-10-27 04:50:23 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/339713008/40001
6 years, 1 month ago (2014-10-27 04:58:31 UTC) #12
commit-bot: I haz the power
6 years, 1 month ago (2014-10-27 06:01:05 UTC) #13
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as 184427

Powered by Google App Engine
This is Rietveld 408576698