63851 2011-07-01 15:04:42 -0700 Crash when loading a document in an editable WebView that has a subframe with an unstyled body 2011-12-21 14:35:45 -0800 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 adele adele eric oldest_to_newest 431478 0 adele 2011-07-01 15:04:42 -0700 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. 431481 1 99528 adele 2011-07-01 15:09:58 -0700 Created attachment 99528 patch 431495 2 99528 darin 2011-07-01 15:53:40 -0700 Comment on attachment 99528 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. 431496 3 99528 adele 2011-07-01 15:57:45 -0700 Comment on attachment 99528 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. 525588 4 eric 2011-12-21 14:29:46 -0800 Attachment 99528 was posted by a committer and has review+, assigning to Adele Peterson for commit. 525622 5 adele 2011-12-21 14:35:45 -0800 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! 99528 2011-07-01 15:09:58 -0700 2011-07-01 15:57:45 -0700 patch patch_crash_editable_view.txt text/plain 2837 adele SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDkwMjc3KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDctMDEgIEFkZWxlIFBl dGVyc29uICA8YWRlbGVAYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIFdlYkNvcmUgcGFydCBvZiBmaXggZm9yIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02Mzg1MQorICAgICAgICA8cmRhcjovL3Byb2JsZW0v ODk4MTA2Mj4gQ3Jhc2ggd2hlbiBsb2FkaW5nIGEgZG9jdW1lbnQgaW4gYW4gZWRpdGFibGUgV2Vi VmlldyB0aGF0IGhhcyBhIHN1YmZyYW1lIHdpdGggYW4gdW5zdHlsZWQgYm9keQorCisgICAgICAg IE5vIG5ldyB0ZXN0cy4gIEkgdHJpZWQgdG8gbWFrZSBhIHRlc3QgaW4gRHVtcFJlbmRlclRyZWUg d2l0aCBhbiBlZGl0YWJsZSBXZWJWaWV3LCAKKyAgICAgICAgYnV0IHdhcyB1bnN1Y2Nlc3NmdWwg aW4gZ2V0dGluZyB0aGUgY3Jhc2ggdG8gaGFwcGVuIGluIHRoYXQgaW5zdGFuY2UuCisKKyAgICAg ICAgKiBlZGl0aW5nL0VkaXRvci5jcHA6IChXZWJDb3JlOjpFZGl0b3I6OmFwcGx5RWRpdGluZ1N0 eWxlVG9FbGVtZW50KTogQWRkIGEgbmlsIGNoZWNrIHRoYXQgZXhpdHMgZWFybHksIGluIGFkZGl0 aW9uIHRvIHRoZSBBU1NFUlQuCisKIDIwMTEtMDctMDEgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdl YmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2phbiBWYWZhaS4KSW5kZXg6IFNvdXJj ZS9XZWJDb3JlL2VkaXRpbmcvRWRpdG9yLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29y ZS9lZGl0aW5nL0VkaXRvci5jcHAJKHJldmlzaW9uIDg4ODUzKQorKysgU291cmNlL1dlYkNvcmUv ZWRpdGluZy9FZGl0b3IuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yODg5LDYgKzI4ODksOCBAQCB2 b2lkIEVkaXRvcjo6YXBwbHlFZGl0aW5nU3R5bGVUb0VsZW1lbnQoCiAKICAgICBDU1NTdHlsZURl Y2xhcmF0aW9uKiBzdHlsZSA9IGVsZW1lbnQtPnN0eWxlKCk7CiAgICAgQVNTRVJUKHN0eWxlKTsK KyAgICBpZiAoIXN0eWxlKQorICAgICAgICByZXR1cm47CiAKICAgICBFeGNlcHRpb25Db2RlIGVj ID0gMDsKICAgICBzdHlsZS0+c2V0UHJvcGVydHkoQ1NTUHJvcGVydHlXb3JkV3JhcCwgImJyZWFr LXdvcmQiLCBmYWxzZSwgZWMpOwpJbmRleDogU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwkocmV2aXNpb24gOTAyNzcp CisrKyBTb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyAr MSwxNCBAQAorMjAxMS0wNy0wMSAgQWRlbGUgUGV0ZXJzb24gIDxhZGVsZUBhcHBsZS5jb20+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2ViS2l0IHBh cnQgb2YgZml4IGZvciBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjM4 NTEKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzg5ODEwNjI+IENyYXNoIHdoZW4gbG9hZGluZyBh IGRvY3VtZW50IGluIGFuIGVkaXRhYmxlIFdlYlZpZXcgdGhhdCBoYXMgYSBzdWJmcmFtZSB3aXRo IGFuIHVuc3R5bGVkIGJvZHkKKworICAgICAgICAqIFdlYlZpZXcvV2ViSFRNTFJlcHJlc2VudGF0 aW9uLm1tOiAoLVtXZWJIVE1MUmVwcmVzZW50YXRpb24gZmluaXNoZWRMb2FkaW5nV2l0aERhdGFT b3VyY2U6XSk6CisgICAgICAgIE9ubHkgY2FsbCBhcHBseUVkaXRpbmdTdHlsZVRvQm9keUVsZW1l bnQgZm9yIHRoZSBtYWluIGZyYW1lLiAgVGhlcmUncyBubyBuZWVkIHRvIGFwcGx5IGJyZWFrLXdv cmQsIAorICAgICAgICBzcGFjZSwgYW5kIGFmdGVyLXdoaXRlLXNwYWNlIHByb3BlcnRpZXMgdG8g c3ViZnJhbWVzIGluIHRoZSBlZGl0YWJsZSBkb2N1bWVudC4KKwogMjAxMS0wNy0wMSAgQW5keSBF c3RlcyAgPGFlc3Rlc0BhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTWFyayBSb3dl LgpJbmRleDogU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJIVE1MUmVwcmVzZW50YXRpb24u bW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJIVE1MUmVwcmVz ZW50YXRpb24ubW0JKHJldmlzaW9uIDg4ODUzKQorKysgU291cmNlL1dlYktpdC9tYWMvV2ViVmll dy9XZWJIVE1MUmVwcmVzZW50YXRpb24ubW0JKHdvcmtpbmcgY29weSkKQEAgLTIyMCw3ICsyMjAs NyBAQCAtICh2b2lkKWZpbmlzaGVkTG9hZGluZ1dpdGhEYXRhU291cmNlOihXCiAgICAgfQogCiAg ICAgV2ViVmlldyAqd2ViVmlldyA9IFt3ZWJGcmFtZSB3ZWJWaWV3XTsKLSAgICBpZiAoW3dlYlZp ZXcgaXNFZGl0YWJsZV0pCisgICAgaWYgKChbd2ViVmlldyBtYWluRnJhbWVdID09IHdlYkZyYW1l KSAmJiBbd2ViVmlldyBpc0VkaXRhYmxlXSkKICAgICAgICAgY29yZSh3ZWJGcmFtZSktPmVkaXRv cigpLT5hcHBseUVkaXRpbmdTdHlsZVRvQm9keUVsZW1lbnQoKTsKIH0KIAo=