Bug 103153 - [CSS Regions] renderer is attached improperly when undo affects a node inside a flow thread
Summary: [CSS Regions] renderer is attached improperly when undo affects a node inside...
Status: RESOLVED DUPLICATE of bug 103501
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Pravin D
URL:
Keywords:
Depends on: 103501
Blocks: 57312
  Show dependency treegraph
 
Reported: 2012-11-23 10:49 PST by Pravin D
Modified: 2012-11-29 02:01 PST (History)
4 users (show)

See Also:


Attachments
TestCase (789 bytes, text/html)
2012-11-23 10:50 PST, Pravin D
no flags Details
Same issue different repro? (1.25 KB, text/html)
2012-11-27 12:41 PST, Andrei Bucur
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pravin D 2012-11-23 10:49:47 PST
Steps to repo:
1) Select all the text inside the red box.
2) Press indent button(once).
3) Press Undo.

Do the same with text inside the green box n compare.

The behavior in the green is correct. The issue is with red box.
Comment 1 Pravin D 2012-11-23 10:50:21 PST
Created attachment 175828 [details]
TestCase
Comment 2 Pravin D 2012-11-23 10:55:11 PST
The issue is due to the fact that in case of text nodes which get inserted into anonymous block, insertbefore does not work properly.


When Indent button is pressed,  the nodes from <div class="artical" contenteditable="true"> are removed in the order

"Non-Workingcase1", <br>, "Non-Workingcase2" and placed inside htmlQuote elem.

 

When Undo button is pressed, the reverse should happen. The content from htmlquote is removed and inserted into the original div, in the reverse order.

(Skipping the node removals)

"Non-Workingcase 2" is first inserted to div. As its a text node, an Anonymous Block is created.

Then  an attempt to insert <br >  before "Non-Workingcase 2 " is made. While doing so "Non-Workingcase 2 " 's renderer is queried  to check if its part of same

flow thread as <div class="artical" contenteditable="true"> . However as the parent of "Non-Workingcase2 " is an Anonymous Block , flow thread info is not set and results in a mismatch. So the renderer before which <br> must be inserted comes null. Which results in <br> being appended to the  Anonymous Block .... {"Non-Workingcase 2 " , <br> } .

When an attemp to insert "Non-Workingcase 1" before <br> is made, it succeeds as <br> does not have any flowthread info, n proper renderer is returmed resulting in {"Non-Workingcase 2 " , "Non-Workingcase 1", <br> } .

Will try n upload a better testcase.
Comment 3 Ryosuke Niwa 2012-11-26 11:51:08 PST
(In reply to comment #2)
> Then  an attempt to insert <br >  before "Non-Workingcase 2 " is made. While doing so "Non-Workingcase 2 " 's renderer is queried  to check if its part of same flow thread as <div class="artical" contenteditable="true">.

Who does this check?
Comment 4 Pravin D 2012-11-27 00:02:37 PST
(In reply to comment #3)
> (In reply to comment #2)
> > Then  an attempt to insert <br >  before "Non-Workingcase 2 " is made. While doing so "Non-Workingcase 2 " 's renderer is queried  to check if its part of same flow thread as <div class="artical" contenteditable="true">.
> 
> Who does this check?
> 

The check is done in nextRenderer() in NodeRenderingContext.cpp .
if (renderer->style() && !renderer->style()->flowThread().isEmpty())
                continue;

However the fix need not be here. I think I'll submit a sample patch, so that we'll have more context to discuss the issue.

Thank you Rniwa
Comment 5 Andrei Bucur 2012-11-27 12:41:55 PST
Created attachment 176328 [details]
Same issue different repro?
Comment 6 Ryosuke Niwa 2012-11-28 11:42:14 PST
I don't understand this bug. Undo/redo code should not be accessing renderer at all other than to check its editability. If you're saying that attach/detach doesn't work properly, then that's a layout bug, not an editing bug.
Comment 7 Pravin D 2012-11-28 21:56:27 PST
(In reply to comment #6)
> I don't understand this bug. Undo/redo code should not be accessing renderer at all other than to check its editability. If you're saying that attach/detach doesn't work properly, then that's a layout bug, not an editing bug.
> 
Yes its an layout and rendering bug only. The component is also set to the same.
Comment 8 Andrei Bucur 2012-11-29 02:01:20 PST

*** This bug has been marked as a duplicate of bug 103501 ***