Bug 219291

Summary: REGRESSION (?): Increasing column-count above 2 at runtime has no effect
Product: WebKit Reporter: Peter Dekkers <peter>
Component: Layout and RenderingAssignee: alan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, niedziolek, pdr, peter, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: All   
OS: Unspecified   
Attachments:
Description Flags
Short video illustrating the buggy behaviour
none
Minimal reproduction
none
Screen-recoding with local fix
none
Patch
none
[fast-cq]Patch none

Peter Dekkers
Reported 2020-11-25 20:03:54 PST
Created attachment 414874 [details] Short video illustrating the buggy behaviour When updating CSS column-count to values above 2 at runtime (i.e. with JavaScript) doesn't seem to take effect. The underlying CSS updates but the browser doesn't display more than two columns. A basic reproduction: https://codepen.io/editkid/pen/WNGNNew I am under the impression that this behaviour only started recently, possibly in Safari 14.
Attachments
Short video illustrating the buggy behaviour (971.92 KB, video/mp4)
2020-11-25 20:03 PST, Peter Dekkers 		no flags
Details
Minimal reproduction (1.74 KB, text/html)
2020-11-25 22:18 PST, Peter Dekkers 		no flags
Details
Screen-recoding with local fix (5.01 MB, video/quicktime)
2023-02-24 22:12 PST, alan 		no flags
Details
Patch (7.31 KB, patch)
2023-02-25 14:55 PST, alan 		no flags
DetailsFormatted DiffDiff
[fast-cq]Patch (9.80 KB, patch)
2023-02-25 15:51 PST, alan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Peter Dekkers
Comment 1 2020-11-25 22:18:38 PST
Created attachment 414881 [details] Minimal reproduction A minimal reproduction of the bug in this simple HTML file with inline JavaScript. To replace the codepen.io link from my previous post.
Radar WebKit Bug Importer
Comment 2 2020-11-30 09:34:36 PST
<rdar://problem/71808738>
Michal Niedziolka
Comment 3 2023-02-23 03:22:24 PST
It's not only about increasing column-count, but simply changing it. Take a look at https://codepen.io/NiedziolkaMichal/pen/MWqeJKb After activating multi-column layout, it's not possible to change the number of columns by changing the value of `columns` or `column-count` property.
alan
Comment 4 2023-02-23 08:27:09 PST
Thank you for adding a minimal repro case, it's really great. and yeah, I am able to reproduce it too. I went back to a relatively old build (from spring 2021) and was still able to repro it. It looks like we don't trigger layout when the property value changes. Forcing layout recovers it (e.g. window resize).
Peter Dekkers
Comment 5 2023-02-23 16:02:02 PST
Thank you. I was hopeful that the insight regarding a forced layout change would enable a workaround for this bug. After trying to trigger a layout change using JavaScript, this doesn't seem possible. Safari blocks most layout changes originating from JavaScript (and, rightfully-so).
alan
Comment 6 2023-02-23 17:07:47 PST
(In reply to Peter Dekkers from comment #5) > Thank you. > > I was hopeful that the insight regarding a forced layout change would enable > a workaround for this bug. After trying to trigger a layout change using > JavaScript, this doesn't seem possible. Safari blocks most layout changes > originating from JavaScript (and, rightfully-so). Oh I just realized this was reported back in 2020 (no wonder the build from 2021 reprod it). :| (apparently it showed up in my mailbox because Michal left a comment). Alight, let me grab this bug.
alan
Comment 7 2023-02-23 17:09:10 PST
> Alight, let me grab this bug. "Alight is a leading cloud-based human capital technology and services provider that powers confident health, wealth and wellbeing decisions for 36 million people and dependents" Not bad, but let's just go with "alright".
Peter Dekkers
Comment 8 2023-02-23 17:10:09 PST
> "Alight is a leading cloud-based human capital technology and services provider Learning every day :) Thank you.
alan
Comment 9 2023-02-24 22:12:49 PST
Created attachment 465167 [details] Screen-recoding with local fix with proper dirty bit propagation, this starts working as expected. will be posting my patch soon.
alan
Comment 10 2023-02-25 14:55:13 PST
Created attachment 465173 [details] Patch
Michal Niedziolka
Comment 11 2023-02-25 15:07:21 PST
Thank you for your very quick response! I have noticed that changes to the `column-width` are also not triggering reflow of the multi-column layout. This fix will affect `column-width` too? If you or somebody else wants to make sure that this patch works well, the following 3 interactive examples should now work well on MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/column-count https://developer.mozilla.org/en-US/docs/Web/CSS/column-width https://developer.mozilla.org/en-US/docs/Web/CSS/columns
Michal Niedziolka
Comment 12 2023-02-25 15:14:45 PST
I think you have a typo in the file dynamic-column-count-change.html at line 11: "document.body.offsetHeigh;" probably should be: "document.body.offsetHeight;"
alan
Comment 13 2023-02-25 15:34:48 PST
(In reply to Michal Niedziolka from comment #12) > I think you have a typo in the file dynamic-column-count-change.html at line > 11: > "document.body.offsetHeigh;" > probably should be: > "document.body.offsetHeight;" good catch, let me fix that.
alan
Comment 14 2023-02-25 15:51:31 PST
Created attachment 465176 [details] [fast-cq]Patch
EWS
Comment 15 2023-02-26 08:12:14 PST
Committed 260849@main (e0b27b7e1c9b): <https://commits.webkit.org/260849@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465176 [details].
Peter Dekkers
Comment 16 2023-02-26 12:56:08 PST
Amazing, cheers @zalan! Columns for everyone.
alan
Comment 17 2023-02-26 19:48:21 PST
(In reply to Peter Dekkers from comment #16) > Amazing, cheers @zalan! Columns for everyone. Yay! [insert the obligatory Oprah meme]
Note You need to log in before you can comment on or make changes to this bug.