Summary: | min-width on a display:table-cell element takes precedence over a width on an outer display:table;table-layout:fixed; element if the outer width is set after initial render | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Phil <pguerrant> | ||||||
Component: | New Bugs | Assignee: | gur.trio | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bdakin, commit-queue, darin, esprehn+autocc, fred.moseley, glenn, gur.trio, hyatt, jarred, k.gurpreet, kling, koivisto, kondapallykalyan, simon.fraser, webkit-bug-importer, zalan | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Phil
2014-03-14 08:27:35 PDT
Created attachment 230978 [details]
Patch
Created attachment 230979 [details]
Patch
Please review. Thanks. Here's an additional twist on what appears to be the same issue. This time there is no min-width involved, just a table/table-cell combo with table-layout:fixed and long non-wrapping text inside, but the symptom is the same - setting the width dynamically does not update the width of the outer element. It always expands to include the width of the text, unless a refresh is triggered by toggling some style (display works) on a timer. <style> #table { display: table; table-layout: fixed; } #cell { display: table-cell; white-space: nowrap; overflow: hidden; } </style> <div id="table"> <div id="cell">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</div> </div> <script> window.onload = function() { table = document.getElementById('table'); table.style.width = '50px'; // workaround: // table.style.display = 'none'; // setTimeout(function() { // table.style.display = ''; // }); }; </script> Hi Phil. The latest content which you mentioned is also working with this patch right? Hi Simon/Darin please review. Comment on attachment 230979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230979&action=review > Source/WebCore/rendering/RenderTable.cpp:105 > + bool fixedTableLayout = oldStyle ? oldStyle->isFixedTableLayout() : false; May as well move this variable down to be above the if() that uses it. (In reply to comment #7) > (From update of attachment 230979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230979&action=review > > > Source/WebCore/rendering/RenderTable.cpp:105 > > + bool fixedTableLayout = oldStyle ? oldStyle->isFixedTableLayout() : false; > > May as well move this variable down to be above the if() that uses it. Is this required? Earlier also the variable was defined at the same place. Hi. Its been more than and no one reviewed this. Please review. Its already landed on blink https://codereview.chromium.org/264283002. Hello. Please review. Hyatt, could you take a look? Do you agree this is correct and the correct approach? Comment on attachment 230979 [details]
Patch
Hi,
Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.
To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.
This issue still exists. What must we do to get it addressed? Alan, Simon, can one of you look over this patch? Committed 260143@main (b6c71868cecc): <https://commits.webkit.org/260143@main> Reviewed commits have been landed. Closing PR #9924 and removing active labels. |