last-child of a list are not styled correctly when the entire list is replaced at once

[David Richards]

Here is a link to a demo:

http://media.rawdod.com/webkit_last_child_regen_bug.html

This is a hacked up copy and paste of the source of: http://www.quirksmode.org/css/firstchild.html

I have confirmed this bug happens on the latest chromium for linux and also the latest safari for windows.

I have confirmed this works fine on the latest firefox for linux.   

I expect nothing platform specific about this bug from that...


From what I am told, webkit should know to reparse the list and style it correctly when you replace the whole list, but it does not. 

This also happens when styling based on the last child of a class, thats how I found it, for example, if I replace this entire admin menu at once, I loose the border-right property being 0 on the last navigation class:

Example of structure:

	ul.navigation {
	
	float: left;
	display: block;
	height: 75px;
	margin: 0 12px 0 2px;
	border-right: 1px dotted #9a9a9a;
	padding: 0 14px 0 0;
	list-style-image: none;	
	list-style-type: none;	
	line-height: 25px;

	}
	
	ul.navigation:last-child {
	
	border-right: 0;
	
	}


<ul class="navigation">
 <li><%= link_to "Realtor Appointments", :controller => "admin/realtorappointments/list" %></li>
 <li><%= link_to "Lender Appointments", :controller => "admin/mortgagelenderappointments/list" %></li>
 <li><%= link_to "Moving Appointments", :controller => "admin/moving_and_storage_appointments/list" %></li>
</ul>
<ul class="navigation">
 <li><%= link_to "Messages", :controller => "admin/messages", :action => "list" %></li>	
 <li><%= link_to "Reports", :controller => "admin/reports", :action => "list" %></li>	
 <li><%= link_to "Recent Logins", :controller => "admin/logs", :action => "recent_logins" %></li>	
</ul>
<ul class="navigation">
 <li><%= link_to "Logs", :controller => "admin/logs", :action => "view" %></li>
 <li><%= link_to "Console", :controller => "admin/logs", :action => "console" %></li>
 <li><%= link_to "Imports", :controller => "admin/imports", :action => "view" %></li>
</ul>
<ul class="navigation">
 <li><%= link_to "Marketing Material Orders", :controller => "admin/logs/material_orders" %></li>
</ul>

[David Richards]

I found some workarounds:

http://media.rawdod.com/webkit_last_child_regen_bug_with_workarounds.html

[David Richards]

This bug is related to 37129  but expands upon it because it also affects other things besides just whats in a list.

[Yoshiki Hayashi]

Created attachment 55823 [details]
Proposed Patch

[Yuzo Fujishima]

Reviewers,

I've uploaded a different patch to:
https://bugs.webkit.org/show_bug.cgi?id=37129

I think we only need either one of the patches. So please review both and pick the one you like. :)

[Kent Tamura]

I read the attachment #55823 [details] (this bug) and #55830 (Bug#37129), and I think the approach of #55823 is better because
 - #55830 uses a parsing flag for non-parsing process
 - #55823 covers not only innerHTML case but also replaceChild(fragment-with-children, node) case

Any other comments?

[Kent Tamura]

Comment on attachment 55823 [details]
Proposed Patch

LayoutTests/fast/css/last-child-innerhtml-expected.txt:6
 +  1
Leaving "1 2 3 1 2 3" in the test expectation makes no much sense.
I recommend to clear children of "threeChildren" and "oneChild" before finishing the test.


WebCore/dom/ContainerNode.cpp:241
 +  
We don't need two blank lines.  Remove one.

[Yoshiki Hayashi]

Created attachment 61341 [details]
Proposed Patch

[Yoshiki Hayashi]

(In reply to comment #6)

Thank you for review.  I've modified the patch accordingly.  Could you take another look?

Thanks,

[Kent Tamura]

Comment on attachment 61341 [details]
Proposed Patch

Looks good.

[WebKit Commit Bot]

Comment on attachment 61341 [details]
Proposed Patch

Clearing flags on attachment: 61341

Committed r63190: <http://trac.webkit.org/changeset/63190>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

*** Bug 37129 has been marked as a duplicate of this bug. ***