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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1128893002: Don't conflate intrinsic width and height with logical width and height (Closed)

Created:
5 years, 7 months ago by davve
Modified:
5 years, 7 months ago
Reviewers:
pdr., kojii, fs
CC:
blink-reviews, blink-reviews-rendering, zoltan1, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, kouhei+svg_chromium.org, fs, Dominik Röttsches, ed+blinkwatch_opera.com, krit, f(malita), jchaffraix+rendering, gyuyoung2, Stephen Chennney, rwlbuis, pdr+svgwatchlist_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Don't conflate intrinsic width and height with logical width and height This is wrong in vertical writing mode and shouldn't be necessary. The intrinsic dimensions are propagated from attributes to properties in SVGSVGElement::collectStyleForPresentationAttribute(). BUG=484425 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195104

Patch Set 1 #

Patch Set 2 : Remove unused method #

Total comments: 2

Patch Set 3 : Rebased for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -25 lines) Patch
D LayoutTests/svg/in-html/sizing/svg-inline-vertical-expected.txt View 1 2 1 chunk +0 lines, -8 lines 0 comments Download
M Source/core/layout/svg/LayoutSVGRoot.cpp View 1 3 chunks +0 lines, -17 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
davve
PTAL. Depends on https://codereview.chromium.org/1128093002/ and https://codereview.chromium.org/1129763002/
5 years, 7 months ago (2015-05-06 13:55:41 UTC) #2
davve
Combined try bot run in https://codereview.chromium.org/1127093002/
5 years, 7 months ago (2015-05-06 13:58:40 UTC) #3
davve
The reason I split this from the other patches was that I saw this as ...
5 years, 7 months ago (2015-05-06 14:16:08 UTC) #4
davve
5 years, 7 months ago (2015-05-07 06:59:41 UTC) #6
fs
lgtm Some notes for future improvement(?): https://codereview.chromium.org/1128893002/diff/20001/Source/core/layout/svg/LayoutSVGRoot.cpp File Source/core/layout/svg/LayoutSVGRoot.cpp (left): https://codereview.chromium.org/1128893002/diff/20001/Source/core/layout/svg/LayoutSVGRoot.cpp#oldcode135 Source/core/layout/svg/LayoutSVGRoot.cpp:135: if (style()->logicalWidth().isSpecified() || ...
5 years, 7 months ago (2015-05-07 11:02:36 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1128893002/40001
5 years, 7 months ago (2015-05-08 06:36:01 UTC) #10
commit-bot: I haz the power
5 years, 7 months ago (2015-05-08 08:00:54 UTC) #11
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195104

Powered by Google App Engine
This is Rietveld 408576698