Bug 157073

Summary: Content disappears on mouse over.
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mmaxfield, rniwa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test reduction
none
Patch
none
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Archive of layout-test-results from ews115 for mac-yosemite
none
Patch
none
Patch
none
Patch
none
Patch none

Description zalan 2016-04-27 09:32:43 PDT
Created attachment 277484 [details]
Test reduction

See test reduction.
Comment 1 zalan 2016-04-27 09:33:24 PDT
rdar://problem/24389168
Comment 2 zalan 2016-04-27 10:05:39 PDT
Created attachment 277486 [details]
Patch
Comment 3 Build Bot 2016-04-27 10:56:18 PDT
Comment on attachment 277486 [details]
Patch

Attachment 277486 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1229698

New failing tests:
fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Comment 4 Build Bot 2016-04-27 10:56:22 PDT
Created attachment 277490 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Simon Fraser (smfr) 2016-04-27 11:04:45 PDT
Comment on attachment 277486 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean)

Missing period at the end.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:973
> +    // removeFromParent() unnecessarily dirties the ancestor subtree.

But do we know that these dirty bits came from the removeFromParent(), or could they have been dirty already?

> Source/WebCore/rendering/RenderText.cpp:1271
> -    // FIXME: should not be needed!!!
> -    if (!textBox.len()) {
> -        // We want the box to be destroyed.
> -        textBox.removeFromParent();
> -        m_lineBoxes.remove(textBox);
> -        delete &textBox;
> -        return;
> -    }
> -
>      m_containsReversedText |= !textBox.isLeftToRightDirection();

This seems like a behavior change. Previously, you would have never hit the last statement if len was 0. Now you do.
Comment 6 Build Bot 2016-04-27 11:05:01 PDT
Comment on attachment 277486 [details]
Patch

Attachment 277486 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1229705

New failing tests:
fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Comment 7 Build Bot 2016-04-27 11:05:05 PDT
Created attachment 277492 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4
Comment 8 Build Bot 2016-04-27 11:11:33 PDT
Comment on attachment 277486 [details]
Patch

Attachment 277486 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1229710

New failing tests:
fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Comment 9 Build Bot 2016-04-27 11:11:38 PDT
Created attachment 277494 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 zalan 2016-04-28 08:31:27 PDT
Created attachment 277622 [details]
Patch
Comment 11 zalan 2016-04-28 08:35:06 PDT
(In reply to comment #5)
> Comment on attachment 277486 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=277486&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean)
> 
> Missing period at the end.
> 
> > Source/WebCore/rendering/RenderBlockLineLayout.cpp:973
> > +    // removeFromParent() unnecessarily dirties the ancestor subtree.
> 
> But do we know that these dirty bits came from the removeFromParent(), or
> could they have been dirty already?
It's a newly constructed line, it's not supposed to be dirty. (fixed)

> 
> > Source/WebCore/rendering/RenderText.cpp:1271
> > -    // FIXME: should not be needed!!!
> > -    if (!textBox.len()) {
> > -        // We want the box to be destroyed.
> > -        textBox.removeFromParent();
> > -        m_lineBoxes.remove(textBox);
> > -        delete &textBox;
> > -        return;
> > -    }
> > -
> >      m_containsReversedText |= !textBox.isLeftToRightDirection();
> 
> This seems like a behavior change. Previously, you would have never hit the
> last statement if len was 0. Now you do.
I assumed a zero length string has no impact on this. (fixed)
Comment 12 zalan 2016-04-28 08:36:02 PDT
Created attachment 277623 [details]
Patch
Comment 13 zalan 2016-04-28 13:17:39 PDT
Created attachment 277644 [details]
Patch
Comment 14 Simon Fraser (smfr) 2016-04-28 13:53:22 PDT
Comment on attachment 277644 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:967
> +#ifndef NDEBUG

Use !ASSERT_DISABLED
Comment 15 zalan 2016-04-28 15:32:35 PDT
Created attachment 277655 [details]
Patch
Comment 16 WebKit Commit Bot 2016-04-28 18:10:48 PDT
Comment on attachment 277655 [details]
Patch

Clearing flags on attachment: 277655

Committed r200220: <http://trac.webkit.org/changeset/200220>
Comment 17 WebKit Commit Bot 2016-04-28 18:10:53 PDT
All reviewed patches have been landed.  Closing bug.