Bug 120953

Summary: Remove unnecessary sibling text renderers after attach
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 120685    
Attachments:
Description Flags
patch
none
fix a few bad test result updates andersca: review+

Description Antti Koivisto 2013-09-07 08:27:12 PDT
The current createTextRenderersForSiblingsAfterAttachIfNeeded code only adds text renderers, never removes them. Style changes can however go either way.
Comment 1 Antti Koivisto 2013-09-07 08:45:26 PDT
Created attachment 210904 [details]
patch
Comment 2 Antti Koivisto 2013-09-07 08:54:38 PDT
Created attachment 210906 [details]
fix a few bad test result updates
Comment 3 Anders Carlsson 2013-09-07 10:58:45 PDT
Comment on attachment 210906 [details]
fix a few bad test result updates

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

> Source/WebCore/ChangeLog:8
> +        The current createTextRenderersForSiblingsAfterAttachIfNeeded code only adds text renderers, never removes them. Style changes can  go either way.

Extra space before "go".

> Source/WebCore/style/StyleResolveTree.cpp:305
> +        bool hadRenderer = !!whitespaceTextSibling.renderer();

Do you really need the !! here?

> LayoutTests/editing/selection/focus-crash-expected.txt:1
> -To run this test manually, click the "Crash me" button. 
> +To run this test manually, click the "Crash me" button.  

What's with this test? Looks like it adds whitespace?

> LayoutTests/userscripts/user-script-all-frames-expected.txt:1
> +If any thingers appear on this page, the test has failed.  

Same here.

> LayoutTests/userscripts/user-script-top-frame-only-expected.txt:1
> +Two thingers should appear on this page. Otherwise the test has failed.  

And here!
Comment 4 Antti Koivisto 2013-09-07 11:01:23 PDT
(In reply to comment #3)
> What's with this test? Looks like it adds whitespace?

Yeah. The new code is also less buggy to other direction. It found a few places where logic dictates we actually need a renderer but failed to create one before.
Comment 5 Antti Koivisto 2013-09-07 11:26:43 PDT
https://trac.webkit.org/r155253