Bug 21287

Summary: Clean up code that changes newStyle in setStyle()
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Description Flags
Patch, test cases, changelog
Phase 2 patch hyatt: review+

Description Simon Fraser (smfr) 2008-10-01 15:50:07 PDT
Various table-related methods muck with the RenderStyle passed into setStyle(). This is evil because
i) that style would be better treated as read-only, and
ii) if someone does a Diff earlier on, their diff gets invalidated.
Comment 1 Simon Fraser (smfr) 2008-10-01 15:52:16 PDT
Created attachment 23997 [details]
Patch, test cases, changelog
Comment 2 Simon Fraser (smfr) 2008-10-01 17:41:18 PDT
Created attachment 23998 [details]
Phase 2 patch
Comment 3 Darin Adler 2008-10-02 13:04:18 PDT
Comment on attachment 23997 [details]
Patch, test cases, changelog

I don't think we should keep RenderTableSection::setStyle around just to do the assertion.

+        if (e && (e->hasTagName(tdTag) || e->hasTagName(thTag))) {

This new code doesn't affect cases where CSS is used to turn an element into a table cell. But the old code did. Is that OK?
Comment 4 Dave Hyatt 2008-10-02 13:07:29 PDT
Comment on attachment 23998 [details]
Phase 2 patch

Comment 5 Simon Fraser (smfr) 2008-10-02 17:43:47 PDT
	M	WebCore/rendering/RenderTableCell.cpp
	M	WebCore/rendering/RenderTableSection.h
	M	WebCore/rendering/RenderTableRow.cpp
	M	WebCore/rendering/RenderTableSection.cpp
	M	WebCore/ChangeLog
	M	WebCore/css/CSSStyleSelector.cpp
	M	LayoutTests/ChangeLog
r37219 = 50e860fa4eefcdaf704355791610209ee4c3fdaa (trunk)

Committed r37220
	M	WebCore/rendering/RenderBlock.cpp
	M	WebCore/rendering/RenderFileUploadControl.cpp
	M	WebCore/rendering/RenderMenuList.cpp
	M	WebCore/ChangeLog
	M	WebCore/css/CSSStyleSelector.cpp
	M	WebCore/css/html4.css
r37220 = 397ffb035ab969a213d69588ea8afcd7e7a3278a (trunk)
Comment 6 Simon Fraser (smfr) 2008-10-02 17:44:09 PDT
Reopen to take (sorry for noise)
Comment 7 Simon Fraser (smfr) 2008-10-03 13:01:11 PDT
Missed the tests first time around:
Committed r37250
	A	LayoutTests/platform/mac/fast/table/table-display-types-expected.txt
	A	LayoutTests/platform/mac/fast/table/floating-th-expected.txt
	A	LayoutTests/platform/mac/fast/table/table-display-types-strict-expected.txt
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/table/table-display-types-strict.html
	A	LayoutTests/fast/table/table-display-types.html
	A	LayoutTests/fast/table/floating-th.html
r37250 = 2473355e15e82022c30a6e765d4e1d7c29e1218e (trunk)
Comment 8 Simon Fraser (smfr) 2008-10-03 13:41:20 PDT
Comment on attachment 23997 [details]
Patch, test cases, changelog

Patch was landed.
Comment 9 Simon Fraser (smfr) 2008-10-03 14:36:28 PDT
Bug 21348 adds a testcase for the behavior change noted in comment 3