Bug 63851 - Crash when loading a document in an editable WebView that has a subframe with an unstyled body
Summary: Crash when loading a document in an editable WebView that has a subframe with...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adele Peterson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-01 15:04 PDT by Adele Peterson
Modified: 2011-12-21 14:35 PST (History)
1 user (show)

See Also:


Attachments
patch (2.77 KB, patch)
2011-07-01 15:09 PDT, Adele Peterson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adele Peterson 2011-07-01 15:04:42 PDT
Crash when loading a document in an editable WebView that has a subframe with a body element with no style

Unfortunately, there's no way for me to test this in Safari, and I was unable to get this to crash in DumpRenderTree.

This patch fixes two problems:
1) In Editor::applyEditingStyleToElement, we assert that there element has style, but in this case I ran into, it did not.  So I added an early return here.
2) In -[WebHTMLRepresentation finishedLoadingWithDataSource:], instead of applying the desirable editing style to any body element, only do it for the main frame.  There's no need to apply break-word, space, and after-white-space properties to subframes in the editable document.
Comment 1 Adele Peterson 2011-07-01 15:09:58 PDT
Created attachment 99528 [details]
patch
Comment 2 Darin Adler 2011-07-01 15:53:40 PDT
Comment on attachment 99528 [details]
patch

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

> Source/WebCore/editing/Editor.cpp:2893
>      CSSStyleDeclaration* style = element->style();
>      ASSERT(style);
> +    if (!style)
> +        return;

This change seems OK, but I’m not sure why it’s needed.

> Source/WebKit/mac/WebView/WebHTMLRepresentation.mm:223
> +    if (([webView mainFrame] == webFrame) && [webView isEditable])

No need for the parentheses here.
Comment 3 Adele Peterson 2011-07-01 15:57:45 PDT
Comment on attachment 99528 [details]
patch

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

>> Source/WebCore/editing/Editor.cpp:2893
>> +        return;
> 
> This change seems OK, but I’m not sure why it’s needed.

Although I couldn't think of specific cases, it seems plausible there would be other situations where there's no style.

>> Source/WebKit/mac/WebView/WebHTMLRepresentation.mm:223
>> +    if (([webView mainFrame] == webFrame) && [webView isEditable])
> 
> No need for the parentheses here.

Will remove.
Comment 4 Eric Seidel (no email) 2011-12-21 14:29:46 PST
Attachment 99528 [details] was posted by a committer and has review+, assigning to Adele Peterson for commit.
Comment 5 Adele Peterson 2011-12-21 14:35:45 PST
Ugh, I forgot to resolve this bug, but I did commit this shortly after the review:  http://trac.webkit.org/changeset/90290

Sorry about that!