New issue
Advanced search Search tips

Issue 873470 link

Starred by 2 users
Status: Fixed
Owner:
f...@opera.com
Closed: Aug 27
Cc:
phanindra.mandapaka@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Console logging an SVGGraphicsElement that is not in DOM, will auto add requiredExtension and systemLanguage attributes with value ""

Reported by tanyalci...@gmail.com, Aug 11 
UserAgent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36

Steps to reproduce the problem:
1. Open the HTML
2. Open the console,keep side by side open
3. Refresh the page, the transformed clone will disappear

What is the expected behavior?
console logging an SVGGraphicsElement that is not yet part of DOM should prevent it from being rendered once it is added.

What went wrong?
- requiredExtensions and systemLanguage attributes are automatically added with value "" which evaluates to 'false' as specified in w3c spec 5.7.1 (https://www.w3.org/TR/SVG/struct.html). 

- However these prevent rendering the element once it is added to the DOM. 

- Removing them at the current tick of event loop does not delete them also, requires a setTimeout/rAF.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 68.0.3440.106  Channel: stable
OS Version: 6.3
Flash Version: 

This question was asked here: https://stackoverflow.com/questions/51785795

and I proposed two temporary solutions here: 
https://stackoverflow.com/questions/51785795#answer-51785921
chromSVG_requiredExtensionSystemLanguage.html
4.0 KB View Download

Comment 1 by swarnasree.mukkala@chromium.org, Aug 12

Labels: Needs-Triage-M68

Comment 2 by schenney@chromium.org, Aug 13

Labels: -Needs-Triage-M68 OS-Android OS-Chrome OS-Linux OS-Mac
Status: Available (was: Unconfirmed)
Seems like we could fix this, though it's not clear how without somehow tracking that the attributes were added just for logging.

Comment 3 by f...@opera.com, Aug 13 

I'd guess that this is caused by attribute synchronization, and the reason console.log triggers it is that it enumerates all properties and thus creates the tear-off/wrapper for systemLanguage/requiredExtensions - which in turn makes them require synching.

Attaching a reduced TC where the console.log dependency has been replaced with a simple access to one of the properties in question.
svg-attr-sync-svgtests.html
374 bytes View Download
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 21 

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

commit a5d95aa93542a4328ddff5b008bafc35b74eb819
Author: Fredrik Söderquist <fs@opera.com>
Date: Tue Aug 21 20:30:16 2018

Rework SVG*TearOff types back-referencing

In preparation for making base-val <-> attribute synchronization,
refactor the SVG*TearOff hierarchy so that "mutation notifications" are
funneled through the SVGAnimatedProperty(Base) if the tear-off is
associated with an attribute.

Rather than storing a SVGElement* and QualifiedName, store a reference
to the underlying SVGAnimatedProperty(Base) instead of the latter. This
is then used to send notification through.

BUG= 873470 

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib6e5c25cf97b806ff613a39478dc4f10f85dcaf5
Reviewed-on: https://chromium-review.googlesource.com/1181342
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#584888}
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/dom/visited_link_state.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/properties/svg_animated_property.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/properties/svg_animated_property.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/properties/svg_list_property_tear_off_helper.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/properties/svg_property_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/properties/svg_property_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_angle_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_angle_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_length_list_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_length_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_length_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_number_list_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_number_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_number_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_point_list_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_point_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_point_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_preserve_aspect_ratio_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_rect_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_rect_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_static_string_list.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_string_list_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_string_list_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_svg_element.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_transform_list_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_transform_list_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_transform_tear_off.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_transform_tear_off.h
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_uri_reference.cc
[modify] https://crrev.com/a5d95aa93542a4328ddff5b008bafc35b74eb819/third_party/blink/renderer/core/svg/svg_uri_reference.h

Comment 5 by phanindra.mandapaka@chromium.org, Aug 23

Cc: phanindra.mandapaka@chromium.org
Labels: Needs-Feedback
Tested this issue on build without fix 70.0.3521.0 using Mac 10.13.6, windows 10 and Ubuntu 17.10 and on the latest chrome version 70.0.3521.0.
Steps:
---------
1. Launched Chrome 
2. Downloaded and Opened the HTML file
3. Opened the console and kept it side by side opened and refreshed the page
We have Observed that the transformed clone is disappear on latest chrome also.

@Reporter: Please find the attached screen-cast and let us know if we have missed anything in the process. Could you please help in verifying the fix. 
Thanks!
873470.mp4
2.1 MB View Download

Comment 6 by tanyalci...@gmail.com, Aug 23 

Just downloaded canary with version 70, I confirm that the problem persists.

Comment 7 by schenney@chromium.org, Aug 23

Labels: -Needs-Feedback
I don't believe this was expected to be fixed. The change was groundwork for fixing the bug.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 24 

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

commit ff29b2308bb305800f5023d0f5e09860483ccb4c
Author: Fredrik Söderquist <fs@opera.com>
Date: Fri Aug 24 15:41:47 2018

Make baseVal attribute synchronization explicit

This moves the |base_value_updated_| flag from the "primitive type"
version of SVGAnimatedProperty to SVGAnimatedPropertyBase, and sets it
in BaseValueChanged. The flag is cleared in SynchronizeAttribute.
This allows some code to be simplified. It also avoids synchronizations
triggered by just having created the tear-off object like in the bug.

Bug:  873470 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I59f30dd69286dc0799a583eb6a52c41022b0af91
Reviewed-on: https://chromium-review.googlesource.com/1188305
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585852}
[add] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/WebKit/LayoutTests/svg/dom/no-value-synching-after-property-get.html
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/properties/svg_animated_property.cc
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/properties/svg_animated_property.h
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_angle.cc
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_angle.h
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_integer_optional_integer.cc
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_integer_optional_integer.h
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_number_optional_number.cc
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_animated_number_optional_number.h
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_static_string_list.cc
[modify] https://crrev.com/ff29b2308bb305800f5023d0f5e09860483ccb4c/third_party/blink/renderer/core/svg/svg_static_string_list.h

Comment 9 by f...@opera.com, Aug 27

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

Sign in to add a comment