[webkit-reviews] review granted: [Bug 89918] [Shadow] Triggers assertion in VisibleSelection::adjustSelectionToAvoidCrossingBoundaries() : [Attachment 149769] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 27 10:04:14 PDT 2012
Ryosuke Niwa <rniwa at webkit.org> has granted Shinya Kawanaka
<shinyak at chromium.org>'s request for review:
Bug 89918: [Shadow] Triggers assertion in
VisibleSelection::adjustSelectionToAvoidCrossingBoundaries()
https://bugs.webkit.org/show_bug.cgi?id=89918
Attachment 149769: Patch
https://bugs.webkit.org/attachment.cgi?id=149769&action=review
------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=149769&action=review
> LayoutTests/ChangeLog:12
> + However, we don't have any test case of selection from Shadow DOM to
elements outside of shadow host.
> + So let's add a testcase here.
I don't think you need to mention this. It's pretty obvious that the reason
we're adding a regression test is because we hadn't had one already.
> LayoutTests/editing/shadow/breaking-editing-boundaries-2.html:6
> +<script src="../../fast/dom/resources/event-sender-util.js"></script>
> +<script src="../../fast/js/resources/js-test-pre.js"></script>
I don't think you need a event-sender-util.js here. You're not using any
functions in there.
Also, I don't see a point in making this a script test. You're not using any
features in there.
> LayoutTests/editing/shadow/breaking-editing-boundaries-2.html:14
> + <div id="host">host</div>
> + <div id="dst">after host</div>
Please spell out destination.
> LayoutTests/editing/shadow/breaking-editing-boundaries-2.html:29
> +var src = shadowRoot.getElementById('src');
> +var dst = document.getElementById('dst');
Ditto about spelling out source and destination.
> LayoutTests/editing/shadow/breaking-editing-boundaries-2.html:37
> + container.innerHTML = "PASS";
This is sufficient without any of js-test-*.js files.
More information about the webkit-reviews
mailing list