WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 77938
SVGTRefElement shouldn't dynamically create a shadow root.
https://bugs.webkit.org/show_bug.cgi?id=77938
Summary
SVGTRefElement shouldn't dynamically create a shadow root.
Shinya Kawanaka
Reported
2012-02-06 22:38:09 PST
It's difficult to support multiple shadow subtrees if a shadow root is recreated. We should change this.
Attachments
Patch
(3.45 KB, patch)
2012-02-07 22:33 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(6.33 KB, patch)
2012-02-09 18:04 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1
2012-02-07 22:33:36 PST
Created
attachment 126005
[details]
Patch
Shinya Kawanaka
Comment 2
2012-02-07 22:45:35 PST
Zimmermann, I'm now implementing "multiple shadow subtrees", which is described in
https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html
When "multiple shadow subtrees" is supported, a user can append a shadow root into any elements. SVG elements are not exceptional. So if a user adds a new shadow root into 'tref' element before tref adds shadow root by itself, a problem may happen. So I would like to ensure that a shadow root is created before user can add a new shadow root into this element. Do you think a problem happens if a shadow root is added in a construction phase? If not, I would like to proceed this change. If it does, we have to think another solution.
Nikolas Zimmermann
Comment 3
2012-02-08 01:46:46 PST
Comment on
attachment 126005
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=126005&action=review
r- for the missing tests for now. Patch looks good in general.
> Source/WebCore/ChangeLog:9 > + SVGTRefElement creates a shadow root dynamically. This will cause a problem to support > + multiple shadow subtrees. So it should be created in a constructor phase. > + > + Reviewed by NOBODY (OOPS!).
These blocks should be swapped. Can you include a testcase that shows this problem? Otherwise this will break again in future.
> Source/WebCore/svg/SVGTRefElement.cpp:61 > - return adoptRef(new SVGTRefElement(tagName, document)); > + RefPtr<SVGTRefElement> elem = adoptRef(new SVGTRefElement(tagName, document)); > + elem->createShadowSubtree(); > + return elem.release();
s/elem/element/ - no abbreviations please.
Shinya Kawanaka
Comment 4
2012-02-08 23:57:49 PST
I'll resubmit this after
Bug 77935
is landed. I'm now using it...
Shinya Kawanaka
Comment 5
2012-02-09 18:04:15 PST
Created
attachment 126417
[details]
Patch
WebKit Review Bot
Comment 6
2012-02-12 17:35:51 PST
Comment on
attachment 126417
[details]
Patch Rejecting
attachment 126417
[details]
from commit-queue.
shinyak@chromium.org
does not have committer permissions according to
http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py
. - If you do not have committer rights please read
http://webkit.org/coding/contributing.html
for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
WebKit Review Bot
Comment 7
2012-02-12 19:35:35 PST
Comment on
attachment 126417
[details]
Patch Clearing flags on attachment: 126417 Committed
r107523
: <
http://trac.webkit.org/changeset/107523
>
WebKit Review Bot
Comment 8
2012-02-12 19:35:40 PST
All reviewed patches have been landed. Closing bug.
Dimitri Glazkov (Google)
Comment 9
2012-05-30 09:41:27 PDT
This bug is still mentioned in
http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/dom/ShadowRoot.cpp&exact_package=chromium&q=allowsAuthorShadowRoot&type=cs&l=75
. Should we remove the check now?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug