Bug 38506 – Regression (r41792): Repaint issue with textarea inside overflow:scroll div

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Steven Lai]

Created attachment 54993 [details]
Test case

Refer to the attached HTML.

There is a textarea inside 2 nested divs, both being position:absolute. Typing text into the textarea results in corrupted text when and only when either scrollLeft or scrollTop of the outer div are > 10.

See the attached screen cap for the corrupted text.

* STEPS TO REPRODUCE
1. Load the html in Safari 4+ or Webkit
2. Type some text into text area

* RESULTS
Text corrupted.

* REGRESSION
Since r41792
http://trac.webkit.org/changeset/41792

* NOTES
If both scrollLeft and scrollTop are < 10, the text is only slightly corrupted.
Already typed texts will be recovered when being repainted, after further scrolling or switching to another window.

<rdar://problem/7100751>

The layout state at the time of repaint has an incorrect offset. It’s a subtree layout and the LayoutState::LayoutState(RenderObject*) constructor, called with the textarea as the root, does not account for the scroll offset of the container (the div).

[James Robinson]

Created attachment 55421 [details]
Test case that doesn't involved textareas

This test case shows that the issue is not limited to text areas but any object relayout boundary.

[James Robinson]

The second test case fails at r41760.  I think that the textarea behavior did regress at r41792 but the underlying bug (the LayoutState ignores the scrolloffset) predates that.

[James Robinson]

Created attachment 55437 [details]
Patch

Comment on attachment 55437 [details]
Patch

> +        RenderLayer* layer = toRenderBoxModelObject(container)->layer();

Although inspection of the current code shows that it’s true, there is no great guarantee that container is a RenderBoxModelObject. Do you think we can change the signature of the constructor to require a RenderBoxModelObject?

Should this also initialize m_clipped and m_clipRect?

[James Robinson]

Thanks for taking a look at this!  I had actually forgotten about this patch completely.

I'll look in to whether to initialize m_clipped and m_clipRect explicitly.  I'm not sure what you mean about changing the signature - do you mean making the constructor something like:

LayoutState(RenderObject* root, RenderBoxModelObject* container)

?  That would just move the cast to the callsite, I believe.

[James Robinson]

Created attachment 57727 [details]
Patch

[James Robinson]

Updated the patch to properly take care of m_clipped and m_clipRect and added a test to cover it.  Without the clip handling we would overpaint in some circumstances.  layout-state-scrolloffset3.html covers this case.

I'm still not quite sure what you mean by changing the signature.

(In reply to comment #6)
> I'm not sure what you mean about changing the signature - do you mean making the constructor something like:
> 
> LayoutState(RenderObject* root, RenderBoxModelObject* container)
> 
> ?  That would just move the cast to the callsite, I believe.

Or perhaps the caller already has a RenderBoxModelObject, or perhaps the caller’s caller, or perhaps just casting there would make it more obvious why the cast is correct; and changing the signature would force new callers to do the right thing.

[James Robinson]

Committed r60640: <http://trac.webkit.org/changeset/60640>