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

Issue 1739533004: Fix synchronization of SVGAnimatedAngle (<marker orient>) (Closed)

Created:
4 years, 9 months ago by fs
Modified:
4 years, 9 months ago
Reviewers:
davve, pdr.
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, chromium-reviews, krit, Eric Willigers, f(malita), gyuyoung2, kouhei+svg_chromium.org, pdr+svgwatchlist_chromium.org, rjwright, rwlbuis, Stephen Chennney, shans
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix synchronization of SVGAnimatedAngle (<marker orient>) Since SVGAnimatedAngle also wraps the SVGAnimatedEnumeration for the enumeration representation of the 'orient' attribute, and both of them synchronize to said attribute, we need to override the synchronization methods to take the synchronization status of them both into account (as opposed to previously where only the SVGAnimatedAngle itself was considered.) Rewrite the existing synchronizeAttribute() implementation to just delegate rather than do the actual work itself. Also change reference from SVGMarkerElement to just SVGElement and include the specific header - SVGAngle.h rather than SVGAngleTearOff.h. BUG=589808 Committed: https://crrev.com/8b5d6fa330a1ffb342db4a72a11982b8fa995722 Cr-Commit-Position: refs/heads/master@{#377875}

Patch Set 1 #

Patch Set 2 : Windows compilation fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -16 lines) Patch
A third_party/WebKit/LayoutTests/svg/dom/SVGMarkerElement-orientType-synchronization.html View 1 chunk +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGAnimatedAngle.h View 1 3 chunks +3 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGAnimatedAngle.cpp View 1 2 chunks +14 lines, -11 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
fs
4 years, 9 months ago (2016-02-26 11:30:14 UTC) #3
davve
Patch looks obviously correct. LGTM. This part of the SVG DOM otoh... *sigh*
4 years, 9 months ago (2016-02-26 12:42:06 UTC) #4
fs
On 2016/02/26 at 12:42:06, davve wrote: > Patch looks obviously correct. LGTM. > > This ...
4 years, 9 months ago (2016-02-26 13:14:00 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1739533004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1739533004/20001
4 years, 9 months ago (2016-02-26 13:14:39 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 9 months ago (2016-02-26 13:19:17 UTC) #8
commit-bot: I haz the power
4 years, 9 months ago (2016-02-26 13:20:24 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/8b5d6fa330a1ffb342db4a72a11982b8fa995722
Cr-Commit-Position: refs/heads/master@{#377875}

Powered by Google App Engine
This is Rietveld 408576698