SVG <use> referencing another <use>, doesn't render properly on change anymore
Reported by
r.a...@panama.de,
Nov 10 2017
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.89 Safari/537.36 Steps to reproduce the problem: 1. Open http://naspa.klassemoji.de/konfigurator/klassemoji/ 2. Scroll down to the configurator interface and select the shoes 3. See the second shoe disappear What is the expected behavior? Shoe should not disappear... What went wrong? Seems like something about the xlink:href resolution changed in chrome 62. Every time, the SVG get's updated in any way, chrome tries to fetch the href async instead of using the referenced source directly from the current page. This process fails, cause it loads the current page (which its the given baseurl) and keeps getting it as text/html. In our case, this happens every time you change a part. Like switching tabs or switching cloth, gender, etc. I will attach a devtool screenshot, that should point out what i mean. This has not happend before chrome 62. Did this work before? Yes Chrome 61 Does this work in other browsers? Yes Chrome version: 62.0.3202.89 Channel: stable OS Version: OS X 10.13.1 Flash Version: We've got multiple instances of configurators like that one, where this problem also happens, with different base url setups. The only case, wich still seems to works, is to not have a <base> tag set and not use baseurl-prefixes on the xlink:href's. But this can cause problems in Firefox, so it's not a proper solution at all. Direct references, like a <use> referencing an <svg> or some child element of an actual <svg> works still fine, just the case <use> referencing another <use> wich is referencing a <svg>-Element fails on a change. (Initial load is fine ...)
,
Nov 10 2017
You're right, you also have to remove the 'ovh' class from the <html> tag to be able to scroll down, sorry for that :/ But anyway, you experienced the issue as well, didn't you?
,
Nov 10 2017
Is it possible to provide a reduced test case? It would save us a lot of time in fixing this.
,
Nov 10 2017
Wow, Firefox on linux is even more horribly broken. Doesn't excuse us, however.
,
Nov 10 2017
So, IIUC, the problem is with a construct like this: <use id="a" href="//some/url/#foo"/> <use href="//some/url/#a"/> (what "foo" actually is isn't relevant I suspect - presumably it exists) and then use#a is mutated to point to #bar (say)? I'll try to make a test along those lines if no one beats me to it...
,
Nov 10 2017
"Is it possible to provide a reduced test case? It would save us a lot of time in fixing this." Sure, if it helps, I'm on it.
,
Nov 10 2017
http://naspa.klassemoji.de/fileadmin/svg-use-issue-testcase.html Does this work for you? Only needed to upload it somewhere cause the issue requires a proper domain, prefixed on the xlink:href's, to appear...
,
Nov 10 2017
That's a good reduction, thanks. Still broken as expected and easy to inspect.
,
Nov 10 2017
Yes, that should do just fine, thanks.
,
Nov 10 2017
,
Nov 10 2017
The bisect in comment #1 is correct.
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01dd29be81769cd9490def490b77c795ccb1202a commit 01dd29be81769cd9490def490b77c795ccb1202a Author: Fredrik Söderquist <fs@opera.com> Date: Fri Nov 10 20:37:29 2017 Fix target reference update for "absolute local" 'href's When the <use> 'href' was specified as an absolute URL, we would not consider it local and attempt to request and use the URL as an "external" resource instead. Because of state inconsistencies and the resource requested being of the wrong type, this would make shadow tree expansion fail for nested <use>. Bug: 783761 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ic4ee946c17e16d5dac96263874792ec3f1d882ea Reviewed-on: https://chromium-review.googlesource.com/763459 Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Fredrik Söderquist <fs@opera.com> Cr-Commit-Position: refs/heads/master@{#515664} [add] https://crrev.com/01dd29be81769cd9490def490b77c795ccb1202a/third_party/WebKit/LayoutTests/http/tests/svg/use-nested-href-absolute-local-reference-expected.html [add] https://crrev.com/01dd29be81769cd9490def490b77c795ccb1202a/third_party/WebKit/LayoutTests/http/tests/svg/use-nested-href-absolute-local-reference.html [modify] https://crrev.com/01dd29be81769cd9490def490b77c795ccb1202a/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
,
Nov 13 2017
I noticed a potentially related issue when performing navigation from-then-to the testcase in the CL, but I'll consider that a separate issue for now.
,
Nov 13 2017
Tanks, for fixing this so fast. We appreciate it. =)
,
Nov 14 2017
,
Nov 14 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 14 2017
This is regressed in M62 (not M63 regression). We're very close to M63 stable promotion so we're only taking critical and safe merges in. How critical is this bug to get fixed in for M63?
,
Nov 14 2017
This is an important and safe merge for M-63. It's been baking for a few days and it fixes functionality that has little to no workaround.
,
Nov 14 2017
Approving merge to M63 branch 3239 based on comment #18. Please merge ASAP so we can take it in for tomorrow's beta release. Thank you.
,
Nov 14 2017
I'll do the merge,
,
Nov 14 2017
Thank you schenney@.
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64065a209cd3aeeef9bf242c816a34ad82e4f91b commit 64065a209cd3aeeef9bf242c816a34ad82e4f91b Author: Fredrik Söderquist <fs@opera.com> Date: Tue Nov 14 19:05:47 2017 Fix target reference update for "absolute local" 'href's M-63 Merge. When the <use> 'href' was specified as an absolute URL, we would not consider it local and attempt to request and use the URL as an "external" resource instead. Because of state inconsistencies and the resource requested being of the wrong type, this would make shadow tree expansion fail for nested <use>. TBR=fs@opera.com Bug: 783761 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ic4ee946c17e16d5dac96263874792ec3f1d882ea Reviewed-on: https://chromium-review.googlesource.com/763459 Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Fredrik Söderquist <fs@opera.com> Cr-Original-Commit-Position: refs/heads/master@{#515664}(cherry picked from commit 01dd29be81769cd9490def490b77c795ccb1202a) Reviewed-on: https://chromium-review.googlesource.com/769367 Cr-Commit-Position: refs/branch-heads/3239@{#484} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [add] https://crrev.com/64065a209cd3aeeef9bf242c816a34ad82e4f91b/third_party/WebKit/LayoutTests/http/tests/svg/use-nested-href-absolute-local-reference-expected.html [add] https://crrev.com/64065a209cd3aeeef9bf242c816a34ad82e4f91b/third_party/WebKit/LayoutTests/http/tests/svg/use-nested-href-absolute-local-reference.html [modify] https://crrev.com/64065a209cd3aeeef9bf242c816a34ad82e4f91b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
,
Nov 29 2017
Issue 789459 has been merged into this issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by woxxom@gmail.com
, Nov 10 2017