Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2706583002:
Fix the way <title> is read under <use> shadow tree (Closed)
|
Created:
3 years, 10 months ago by mrunal Modified:
3 years, 9 months ago Reviewers:
fs CC:
fs, blink-reviews, chromium-reviews, krit, f(malita), gyuyoung2, kouhei+svg_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix the way <title> is read under <use> shadow tree
Earlier there was no way <title> element could be read under <use> shadow
tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any.
Otherwise we return the first <title> child from the shadow tree instance under <use>.
BUG=619073
Review-Url: https://codereview.chromium.org/2706583002
Cr-Commit-Position: refs/heads/master@{#454683}
Committed: https://chromium.googlesource.com/chromium/src/+/58b15addedd3cb872fedf1d7a9c88fce8c35e0de
Patch Set 1 #
Total comments: 4
Patch Set 2 : Add title() handler to SVGUseElement and remove code from SVGElement #
Total comments: 1
Patch Set 3 : Add layout test #
Total comments: 7
Patch Set 4 : Add testharness based test case and remove shadow tree test case #
Total comments: 13
Patch Set 5 : Add checks for eventSender and testRunner #
Total comments: 4
Patch Set 6 : Remove the validity checks for eventSender and testRunner #
Messages
Total messages: 32 (17 generated)
Description was changed from ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. This commit uses FlatTreeTraversal incase there is no immediate <title> element under regular <use> tree to search for <use> shadow tree. It returns the first <title> found in the shadow tree. BUG=619073 ========== to ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. This commit uses FlatTreeTraversal incase there is no immediate <title> element under regular <use> tree to search for <use> shadow tree. It returns the first <title> found in the shadow tree. BUG=619073 ==========
mrunal.kapade@intel.com changed reviewers: + fs@opera.com
Hi fs, Can you PTAL. Thanks.
You should add some form of test for this (there's a 'tooltipText' or so on testRunner that could be used for this; a unit test would also work.) https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/svg/SVGElement.cpp (right): https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/svg/SVGElement.cpp:206: // find the first <title> child of this element and return it's text contents. Nit: its https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/svg/SVGElement.cpp:210: // descendants using FlatTreeTraversal. I don't think we want to look at _all_ descendants - we want to limit ourselves to direct children. For <use> that'll probably mean "direct children of the referenced element". This is not what the reporter of the bug was requesting, but I think it's more in line with the a11y guidelines/name computations I've seen [1], so it may be better to follow that? Regardless, maybe SVGUseElement should just have its own override for title() and inspect the shadow tree directly? (Avoiding a flat tree traversal.) [1] https://www.w3.org/TR/svg-aam-1.0/#mapping_additional_nd
https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/svg/SVGElement.cpp (right): https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/svg/SVGElement.cpp:210: // descendants using FlatTreeTraversal. On 2017/02/24 09:06:18, fs wrote: > I don't think we want to look at _all_ descendants - we want to limit ourselves > to direct children. For <use> that'll probably mean "direct children of the > referenced element". This is not what the reporter of the bug was requesting, The example in the bug uses <title> which is nested inside <rect> which inturn is nested inside <symbol>. So are you suggesting we limit ourselves to <title> only if it's inside <symbol> or other top level referenced element? > but I think it's more in line with the a11y guidelines/name computations I've > seen [1], so it may be better to follow that? > Regardless, maybe SVGUseElement should just have its own override for title() > and inspect the shadow tree directly? (Avoiding a flat tree traversal.) I also thought about that solution in the beginning but I don't think you can still use simple ElementTraversal in SVGUselement as you are still one level above shadow root making ElementTaversal useless unless you somehow propagate the <title> at the same level as shadow-root while building it. Unless there is another way? > > [1] https://www.w3.org/TR/svg-aam-1.0/#mapping_additional_nd
https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/svg/SVGElement.cpp (right): https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/svg/SVGElement.cpp:210: // descendants using FlatTreeTraversal. On 2017/02/25 at 00:31:18, mrunal wrote: > On 2017/02/24 09:06:18, fs wrote: > > I don't think we want to look at _all_ descendants - we want to limit ourselves > > to direct children. For <use> that'll probably mean "direct children of the > > referenced element". This is not what the reporter of the bug was requesting, > The example in the bug uses <title> which is nested inside <rect> which inturn is nested inside <symbol>. So are you suggesting we limit ourselves to <title> only if it's inside <symbol> or other top level referenced element? Yes, per the referenced document [1]. > > but I think it's more in line with the a11y guidelines/name computations I've > > seen [1], so it may be better to follow that? > > Regardless, maybe SVGUseElement should just have its own override for title() > > and inspect the shadow tree directly? (Avoiding a flat tree traversal.) > I also thought about that solution in the beginning but I don't think you can still use simple ElementTraversal in SVGUselement as you are still one level above shadow root making ElementTaversal useless unless you somehow propagate the <title> at the same level as shadow-root while building it. Unless there is another way? If you are in SVGUseElement you can trivially access the shadow root (because you know it's there) and "descend" into the shadow tree (also per [1].) > > [1] https://www.w3.org/TR/svg-aam-1.0/#mapping_additional_nd
On 2017/02/27 08:57:09, fs wrote: > https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/svg/SVGElement.cpp (right): > > https://codereview.chromium.org/2706583002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/svg/SVGElement.cpp:210: // descendants using > FlatTreeTraversal. > On 2017/02/25 at 00:31:18, mrunal wrote: > > On 2017/02/24 09:06:18, fs wrote: > > > I don't think we want to look at _all_ descendants - we want to limit > ourselves > > > to direct children. For <use> that'll probably mean "direct children of the > > > referenced element". This is not what the reporter of the bug was > requesting, > > The example in the bug uses <title> which is nested inside <rect> which inturn > is nested inside <symbol>. So are you suggesting we limit ourselves to <title> > only if it's inside <symbol> or other top level referenced element? > > Yes, per the referenced document [1]. > > > > but I think it's more in line with the a11y guidelines/name computations > I've > > > seen [1], so it may be better to follow that? > > > Regardless, maybe SVGUseElement should just have its own override for > title() > > > and inspect the shadow tree directly? (Avoiding a flat tree traversal.) > > I also thought about that solution in the beginning but I don't think you can > still use simple ElementTraversal in SVGUselement as you are still one level > above shadow root making ElementTaversal useless unless you somehow propagate > the <title> at the same level as shadow-root while building it. Unless there is > another way? > > If you are in SVGUseElement you can trivially access the shadow root (because > you know it's there) and "descend" into the shadow tree (also per [1].) > > > > [1] https://www.w3.org/TR/svg-aam-1.0/#mapping_additional_nd Thanks for your helpful suggestions. I have now added title() override to SVGUseElement instead. Once you think it's fine I will update the commit message to reflect the new logic. In the meantime I will work on adding the test. Do you need both, Layout Test as well as Unit Test?
Description was changed from ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. This commit uses FlatTreeTraversal incase there is no immediate <title> element under regular <use> tree to search for <use> shadow tree. It returns the first <title> found in the shadow tree. BUG=619073 ========== to ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ==========
Description was changed from ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ========== to ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any. Otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ==========
Description was changed from ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any. Otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ========== to ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any. Otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ==========
On 2017/02/28 at 01:07:14, mrunal.kapade wrote: ... > Thanks for your helpful suggestions. I have now added title() override to SVGUseElement instead. > Once you think it's fine I will update the commit message to reflect the new logic. > In the meantime I will work on adding the test. Do you need both, Layout Test as well as Unit Test? Code-change looks good. No need for both, either one will be fine. https://codereview.chromium.org/2706583002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): https://codereview.chromium.org/2706583002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/svg/SVGUseElement.cpp:354: // Otherwise return a null/empty string. null/empty -> null (String() is a null string, for an empty string use emptyString.)
The CQ bit was checked by mrunal.kapade@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/02/28 09:07:15, fs wrote: > On 2017/02/28 at 01:07:14, mrunal.kapade wrote: > ... > > Thanks for your helpful suggestions. I have now added title() override to > SVGUseElement instead. > > Once you think it's fine I will update the commit message to reflect the new > logic. > > In the meantime I will work on adding the test. Do you need both, Layout Test > as well as Unit Test? > > Code-change looks good. No need for both, either one will be fine. I have added layout test next to the existing title-in-shadow-tree.html test. > > https://codereview.chromium.org/2706583002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): > > https://codereview.chromium.org/2706583002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/svg/SVGUseElement.cpp:354: // Otherwise return a > null/empty string. > null/empty -> null > > (String() is a null string, for an empty string use emptyString.) Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html (right): https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html:11: </use> Nit: Can move this to the previous line. https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html:16: document.body.createShadowRoot().appendChild(template.content.cloneNode(true)); Should test this as a regular <use> (outside of an shadow explicit), which is what the bug is about. Having a test like this is a good bonus though. https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html:24: testRunner.dumpAsText(); It should be possible (and preferable) to write this as a testharness.js test. https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/svg/SVGUseElement.cpp:351: Traversal<SVGTitleElement>::firstChild(*m_targetElementInstance)) m_targetElementInstance can be null here, so that needs to be handled.
https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html (right): https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html:24: testRunner.dumpAsText(); On 2017/03/01 09:29:05, fs wrote: > It should be possible (and preferable) to write this as a testharness.js test. Done. I have removed this test and added testharness.js test now. I adapted this from title-in-shadow-tree.html test thinking it was testing similar things but looks like it is different. BTW, I also saw third_party/WebKit/ManualTests/svg-tooltip.svg as part of manual tests which should also cover this but I am not sure how Manual Tests are run. https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/svg/SVGUseElement.cpp:351: Traversal<SVGTitleElement>::firstChild(*m_targetElementInstance)) On 2017/03/01 09:29:05, fs wrote: > m_targetElementInstance can be null here, so that needs to be handled. Done.
The CQ bit was checked by mrunal.kapade@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Thanks, looks good for the most part, just needs a few smaller fixups before it's good to go. https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html (right): https://codereview.chromium.org/2706583002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/title-in-shadow-tree-without-any-title-in-use.html:24: testRunner.dumpAsText(); On 2017/03/02 at 03:37:43, mrunal wrote: > On 2017/03/01 09:29:05, fs wrote: > > It should be possible (and preferable) to write this as a testharness.js test. > > Done. I have removed this test and added testharness.js test now. I adapted this from title-in-shadow-tree.html test thinking it was testing similar things but looks like it is different. > BTW, I also saw third_party/WebKit/ManualTests/svg-tooltip.svg as part of manual tests which should also cover this but I am not sure how Manual Tests are run. ManualTests are manually, so I guess in general they aren't run at all =). (And hence they are of lesser value in general...) https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html (right): https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:2: <body> You don't need this. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:5: <div id=log></div> Or this. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:13: <symbol id="redSquare"> Nit: Could you make this "blueSquare" or so instead ("red" is a common indicator of failure, so it's good to avoid using that color when it doesn't convey failure.) https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:30: eventSender.dragMode = false; But these two lines in the test function (this should make the test fail more gracefully when the eventSender isn't available.) https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:37: </body> Since <body> isn't needed, neither is this. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/svg/SVGUseElement.cpp:350: if (!m_targetElementInstance) { Reverse condition.
https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html (right): https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:2: <body> On 2017/03/02 08:45:07, fs wrote: > You don't need this. Done. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:5: <div id=log></div> On 2017/03/02 08:45:07, fs wrote: > Or this. Done. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:13: <symbol id="redSquare"> On 2017/03/02 08:45:07, fs wrote: > Nit: Could you make this "blueSquare" or so instead ("red" is a common indicator > of failure, so it's good to avoid using that color when it doesn't convey > failure.) Done. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:30: eventSender.dragMode = false; On 2017/03/02 08:45:07, fs wrote: > But these two lines in the test function (this should make the test fail more > gracefully when the eventSender isn't available.) Not sure what you meant here but I have now added checks for eventSender and testRunner. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:37: </body> On 2017/03/02 08:45:07, fs wrote: > Since <body> isn't needed, neither is this. Done. https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (right): https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/svg/SVGUseElement.cpp:350: if (!m_targetElementInstance) { On 2017/03/02 08:45:07, fs wrote: > Reverse condition. Oops..I mistakenly replaced the other way I was planning on checking the m_targetElementInstance. Good catch.
LGTM w/ nits https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html (right): https://codereview.chromium.org/2706583002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:30: eventSender.dragMode = false; On 2017/03/02 at 23:49:07, mrunal wrote: > On 2017/03/02 08:45:07, fs wrote: > > But these two lines in the test function (this should make the test fail more > > gracefully when the eventSender isn't available.) > > Not sure what you meant here but I have now added checks for eventSender and testRunner. What I meant was to write this as: test(function() { eventSender.dragMode = false; ... }, description); instead, since these actions are a part of the test. (And since these will throw if eventSender isn't available it's good to have them within the test() body, since the test harness will catch and report that.) Not a huge deal, but preferable. https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html (right): https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:12: <rect width="50" height="50" fill="red"> Nit: red -> blue https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:24: test(function () { assert_unreached(); }, "Cannot run tests without eventSender and testRunner"); If you move the eventSender manipulation inside the test() function, this will not be needed. Conditional tests like this are not a great idea, so if one were to include something like this, it'd be better to do it as: test(function() { assert_exists(window, eventSender); assert_exists(window, testRunner); }, 'Test environment'); (or use setup() instead of test().) Better to leave it out though I think, so please remove.
https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html (right): https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:12: <rect width="50" height="50" fill="red"> On 2017/03/03 08:53:30, fs wrote: > Nit: red -> blue Aahh...Done. Thanks. https://codereview.chromium.org/2706583002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/svg/dom/tooltip-title-with-use.html:24: test(function () { assert_unreached(); }, "Cannot run tests without eventSender and testRunner"); On 2017/03/03 08:53:30, fs wrote: > If you move the eventSender manipulation inside the test() function, this will > not be needed. Conditional tests like this are not a great idea, so if one were > to include something like this, it'd be better to do it as: > > test(function() { > assert_exists(window, eventSender); > assert_exists(window, testRunner); > }, 'Test environment'); > > (or use setup() instead of test().) Better to leave it out though I think, so > please remove. Ok, removed.
The CQ bit was checked by mrunal.kapade@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com Link to the patchset: https://codereview.chromium.org/2706583002/#ps100001 (title: "Remove the validity checks for eventSender and testRunner")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488571812246660,
"parent_rev": "5acfe365a2d66040792eb951bf919ae178deb860", "commit_rev":
"58b15addedd3cb872fedf1d7a9c88fce8c35e0de"}
Message was sent while issue was closed.
Description was changed from ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any. Otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 ========== to ========== Fix the way <title> is read under <use> shadow tree Earlier there was no way <title> element could be read under <use> shadow tree. By implementing title() override in SVGUseElement we return first <title> child under <use> if there is any. Otherwise we return the first <title> child from the shadow tree instance under <use>. BUG=619073 Review-Url: https://codereview.chromium.org/2706583002 Cr-Commit-Position: refs/heads/master@{#454683} Committed: https://chromium.googlesource.com/chromium/src/+/58b15addedd3cb872fedf1d7a9c8... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/58b15addedd3cb872fedf1d7a9c8... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
