RESOLVED FIXED 21287
Clean up code that changes newStyle in setStyle()
https://bugs.webkit.org/show_bug.cgi?id=21287
Summary Clean up code that changes newStyle in setStyle()
Simon Fraser (smfr)
Reported 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.
Attachments
Patch, test cases, changelog (33.22 KB, patch)
2008-10-01 15:52 PDT, Simon Fraser (smfr)
no flags
Phase 2 patch (3.89 KB, patch)
2008-10-01 17:41 PDT, Simon Fraser (smfr)
hyatt: review+
Simon Fraser (smfr)
Comment 1 2008-10-01 15:52:16 PDT
Created attachment 23997 [details] Patch, test cases, changelog
Simon Fraser (smfr)
Comment 2 2008-10-01 17:41:18 PDT
Created attachment 23998 [details] Phase 2 patch
Darin Adler
Comment 3 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?
Dave Hyatt
Comment 4 2008-10-02 13:07:29 PDT
Comment on attachment 23998 [details] Phase 2 patch r=me
Simon Fraser (smfr)
Comment 5 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)
Simon Fraser (smfr)
Comment 6 2008-10-02 17:44:09 PDT
Reopen to take (sorry for noise)
Simon Fraser (smfr)
Comment 7 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)
Simon Fraser (smfr)
Comment 8 2008-10-03 13:41:20 PDT
Comment on attachment 23997 [details] Patch, test cases, changelog Patch was landed.
Simon Fraser (smfr)
Comment 9 2008-10-03 14:36:28 PDT
Bug 21348 adds a testcase for the behavior change noted in comment 3
Note You need to log in before you can comment on or make changes to this bug.