Bug 157073 – Content disappears on mouse over.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

Created attachment 277484 [details]
Test reduction

See test reduction.

[zalan]

rdar://problem/24389168

[zalan]

Created attachment 277486 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Simon Fraser (smfr)]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[zalan]

Created attachment 277622 [details]
Patch

[zalan]

(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)

[zalan]

Created attachment 277623 [details]
Patch

[zalan]

Created attachment 277644 [details]
Patch

[Simon Fraser (smfr)]

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

[zalan]

Created attachment 277655 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 277655 [details]
Patch

Clearing flags on attachment: 277655

Committed r200220: <http://trac.webkit.org/changeset/200220>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.