Summary: | Setting valign on a table in JavaScript has no effect | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brett Wilson (Google) <brettw> | ||||||||||
Component: | Tables | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | NEW --- | ||||||||||||
Severity: | Normal | CC: | ahmad.saleem792, hyatt, mitz | ||||||||||
Priority: | P2 | Keywords: | HasReduction | ||||||||||
Version: | 523.x (Safari 3) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Brett Wilson (Google)
2007-08-01 09:18:21 PDT
Created attachment 15781 [details]
Testcase
Pressing the button should valign the rows, but does not. It works in IE and Firefox.
Created attachment 15788 [details]
Test case for the underlying bug.
vertical-align is not an inherited property, so recalcStyle does not continue to descendants. What recalcStyle does not know is that table rows and table cells have 'vertical-align: inherit' by default, so they do need a recalc.
The issue is not limited to mapped attributes, as this test case demonstrates.
We basically need to find a way to retain the (very useful) performance optimization of avoiding propagation to decendants for non-inherited properties while still being correct. One idea would be to just propagate non-inherited properties when we see an inherit keyword used on any stylesheet used by the document. As discussed on IRC: this would mean adding a HashSet of properties which used the inherit keyword at any time during the document's lifetime. We would do a lookup in that set after setting any non-inherited CSS property, if the lookup returned true, then we'd force a style re-calc just as though the property was an inherited property. We can also just hack valign in the table DOM code. Created attachment 58983 [details]
Proposed Patch
If vAlign property of the table row or table cell is changed through javascript,this patch will propagate the effect down the hierarchy.
Created attachment 59661 [details]
Proposed Path with change log
If vAlign property of the table row or table cell is changed through javascript,this patch will propagate the effect down the hierarchy.
Comment on attachment 58983 [details]
Proposed Patch
Removing r? from redundant patch.
Comment on attachment 59661 [details]
Proposed Path with change log
I'm not sure about the correctness of this code, but it needs a layout test before it can be committed.
We pass all valign test cases - https://wpt.fyi/results/html?label=master&label=experimental&aligned&view=subtest&q=valign Do we need to do anything more here? |