Bug 75851 – NULL ptr in WebCore::RenderSVGInlineText::localCaretRect

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Berend-Jan Wever]

Created attachment 121659 [details]
Repro

Chromium: http://code.google.com/p/chromium/issues/detail?id=109607

Repro:
<svg xmlns="http://www.w3.org/2000/svg"><style>
  * {
    border-top-style: inset
  }
</style><script>
  window.onload=function() {
    _Selection_0=getSelection();
    _Selection_0.setBaseAndExtent(document,5,document,5);
    _Selection_0.deleteFromDocument();
    _Selection_0.modify('extend','backward','line');
  }
</script>
<text></text><text> 1

[Stephen Chenney]

Created attachment 122495 [details]
Patch

Every other implementation of localCaretRect(...) checks for a null box. This makes the SVG inline text do the same, which fixes the reported chromium crash. I was unable to create a LayoutTest that crashed, so there is none added.

[Ryosuke Niwa]

Comment on attachment 122495 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122495&action=review

> Source/WebCore/ChangeLog:10
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the description.

[Ryosuke Niwa]

Kelly, please see my review comment. "Reviewed by" line in the change log is inserted at the wrong location and that's why I didn't cq+ it.

[Stephen Chenney]

Created attachment 122517 [details]
Patch

Fixed review line location. Apologies for not fixing pre-commit.

[Ryosuke Niwa]

Wait a minute, you didn't a test. Please add a new layout test.

[Stephen Chenney]

I could not figure out how to make LayoutTestController generate this stack trace, which is the crash stack trace. I'll try some more, but any help would be appreciated.


#0  0x00007ffff396646c in WebCore::RenderSVGInlineText::localCaretRect (this=0x7fffe2494658, box=0x0, caretOffset=0)
    at third_party/WebKit/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp:141
#1  0x00007ffff314d9d3 in WebCore::RenderedPosition::absoluteRect (this=0x7fffffff9b90, extraWidthToEndOfLine=
    0x7fffffff9cec) at third_party/WebKit/Source/WebCore/editing/RenderedPosition.cpp:232
#2  0x00007ffff3130cfb in WebCore::RenderedPosition::absoluteRect (this=0x7fffffff9b90, extraWidthToEndOfLine=
    @0x7fffffff9cec) at third_party/WebKit/Source/WebCore/editing/RenderedPosition.h:71
#3  0x00007ffff312d097 in WebCore::Editor::firstRectForRange (this=0x7fffe23feef8, range=0x7fffe14b4600)
    at third_party/WebKit/Source/WebCore/editing/Editor.cpp:2640
#4  0x00007ffff294cd02 in WebKit::WebViewImpl::selectionBounds (this=0x7fffefe22600, start=..., end=...)
    at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:1674
#5  0x00007ffff43f4823 in RenderWidget::GetSelectionBounds (this=0x7fffefe53000, start=0x7fffffff9fa0, end=
    0x7fffffff9f90) at content/renderer/render_widget.cc:1454
#6  0x00007ffff43dcfcd in RenderViewImpl::GetSelectionBounds (this=0x7fffefe53000, start=0x7fffffff9fa0, end=
    0x7fffffff9f90) at content/renderer/render_view_impl.cc:4551
#7  0x00007ffff43f48ea in RenderWidget::UpdateSelectionBounds (this=0x7fffefe53000)
    at content/renderer/render_widget.cc:1465
#8  0x00007ffff43f204e in RenderWidget::DoDeferredUpdate (this=0x7fffefe53000) at content/renderer/render_widget.cc:908
#9  0x00007ffff43f0b43 in RenderWidget::DoDeferredUpdateAndSendInputAck (this=0x7fffefe53000)
    at content/renderer/render_widget.cc:730
#10 0x00007ffff43eee2c in RenderWidget::OnUpdateRectAck (this=0x7fffefe53000) at content/renderer/render_widget.cc:383

[Stephen Chenney]

Created attachment 122807 [details]
Patch

Adding a manual test case, as the issue was not reproducable with DRT.

[Stephen Chenney]

Damn. Crash isn't happening with this test - swear it was.

[Stephen Chenney]

Created attachment 122810 [details]
Patch

This test file crashes.

[Ryosuke Niwa]

Comment on attachment 122810 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122810&action=review

It's still sad that we can't add an automated test.

> ManualTests/svg-modify-deleted-selection.svg:12
> +    _Selection_0=getSelection();
> +    _Selection_0.setBaseAndExtent(document,5,document,5);
> +    _Selection_0.deleteFromDocument();
> +    _Selection_0.modify('extend','backward','line');

Why do we need _Selection_0? You can just call getSelection() on every line.

[Stephen Chenney]

Created attachment 122824 [details]
Patch

Got rid of the _Selection_0 variable.

[WebKit Review Bot]

Comment on attachment 122824 [details]
Patch

Clearing flags on attachment: 122824

Committed r105224: <http://trac.webkit.org/changeset/105224>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.