New issue
Advanced search Search tips

Issue 843901 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 28
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Removing an item from rect (SVGTransformList) prevents attribute removal

Reported by bret...@gmail.com, May 17 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce the problem:
Run the attached file.

(The following code:

var elem = document.createElementNS('http://www.w3.org/2000/svg', 'rect');
elem.setAttribute('transform', 'matrix(1,0,0,1,0,0)');
elem.transform.baseVal.removeItem(0);
elem.removeAttribute('transform');
console.log(elem.hasAttribute('transform'));

)

What is the expected behavior?
Get `false`

What went wrong?
Gets `true`

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 66.0.3359.139  Channel: stable
OS Version: OS X 10.13.4
Flash Version: 

Will remove the attribute if `removeItem` is not called.

This works correctly in Firefox.
 
removeItem-bug.html
507 bytes View Download

Comment 1 by f...@opera.com, May 17 2018

Components: Blink>SVG
Labels: OS-Android OS-Chrome OS-Linux OS-Windows
Status: Available (was: Unconfirmed)
This is a variation of  issue 589856 . In this case though I think we're just missing an attribute synchronization in removeAttribute(...). What the state should've been after removeItem(0) ("" or no attribute) doesn't appear to be fully/well specified, so I raised a spec issue on that.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7ddd4ac2f51c6957cd77a585b5bc9ddc1c6a0bc

commit c7ddd4ac2f51c6957cd77a585b5bc9ddc1c6a0bc
Author: Fredrik Söderquist <fs@opera.com>
Date: Mon Aug 27 13:53:24 2018

Clear SVG property synchronization flag on attribute update

When an SVGAnimatedProperty has its associated attribute updated, we can
cancel any pending attribute synchronization.
This prevents overwriting the new value with the old in some cases.

Bug:  843901 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3654ad616fd6e6ef82a1933ec034bc9cf0097d56
Reviewed-on: https://chromium-review.googlesource.com/1189842
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#586246}
[add] https://crrev.com/c7ddd4ac2f51c6957cd77a585b5bc9ddc1c6a0bc/third_party/WebKit/LayoutTests/svg/dom/no-value-synching-after-attr-removal.html
[modify] https://crrev.com/c7ddd4ac2f51c6957cd77a585b5bc9ddc1c6a0bc/third_party/blink/renderer/core/svg/properties/svg_animated_property.h
[modify] https://crrev.com/c7ddd4ac2f51c6957cd77a585b5bc9ddc1c6a0bc/third_party/blink/renderer/core/svg/svg_static_string_list.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a216c3df2e588469aa0b6b8cccfd70660f4d3688

commit a216c3df2e588469aa0b6b8cccfd70660f4d3688
Author: Fredrik Söderquist <fs@opera.com>
Date: Mon Aug 27 14:17:18 2018

Empty SVG*List should serialize to the null String

When synchronizing an empty list, the result should be that the
attribute is removed. However, since StringBuilder::ToString for an
"empty" StringBuilder returns the empty string we end up setting an
empty attribute. Add a special case to handle empty lists, and return
the null String there instead. The null String will translate to an
attribute removal.

All the SVG*List types that inherit from SVGListPropertyHelper are
changed call a new shared implementation (SerializeList) in that class.

Bug:  843901 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I9442268196aaa605886fd4d6d31d66ae075f901d
Reviewed-on: https://chromium-review.googlesource.com/1189862
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#586251}
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/WebKit/LayoutTests/svg/dom/no-value-synching-after-attr-removal.html
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/properties/svg_list_property_helper.h
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/svg_length_list.cc
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/svg_number_list.cc
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/svg_point_list.cc
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/svg_string_list.cc
[modify] https://crrev.com/a216c3df2e588469aa0b6b8cccfd70660f4d3688/third_party/blink/renderer/core/svg/svg_transform_list.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e49d75377a189c71e1deb1ef41da3cb9269387c

commit 3e49d75377a189c71e1deb1ef41da3cb9269387c
Author: Fredrik Söderquist <fs@opera.com>
Date: Tue Aug 28 09:21:16 2018

Call CommitChange after a SVG*List.clear() operation

The SVGListPropertyTearOffHelper did not call CommitChange(), meaning
that the element wasn't notified of the change, and wouldn't invalidate
et.c as needed. It also could've failed to synchronize the attribute
properly.

Bug:  843901 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iad581027c3748e3e90cf6fe3d944273b207fef0f
Reviewed-on: https://chromium-review.googlesource.com/1189882
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586625}
[modify] https://crrev.com/3e49d75377a189c71e1deb1ef41da3cb9269387c/third_party/WebKit/LayoutTests/svg/dom/no-value-synching-after-attr-removal.html
[add] https://crrev.com/3e49d75377a189c71e1deb1ef41da3cb9269387c/third_party/WebKit/LayoutTests/svg/dom/svgtransformlist-clear-invalidates-expected.html
[add] https://crrev.com/3e49d75377a189c71e1deb1ef41da3cb9269387c/third_party/WebKit/LayoutTests/svg/dom/svgtransformlist-clear-invalidates.html
[modify] https://crrev.com/3e49d75377a189c71e1deb1ef41da3cb9269387c/third_party/blink/renderer/core/svg/properties/svg_list_property_tear_off_helper.h

Owner: f...@opera.com
Status: Fixed (was: Available)

Sign in to add a comment