|Summary:||[CSS Regions] webkitregionlayoutupdate is triggered for all flows even though only one flow has to be updated|
|Product:||WebKit||Reporter:||Johannes Wilm <johanneswilm>|
|Component:||Web Inspector (Deprecated)||Assignee:||Andrei Bucur <abucur>|
|Severity:||Normal||CC:||abucur, apavlov, eric, esprehn+autocc, keishi, loislo, mibalan, mihnea, ojan.autocc, pfeldman, pmuellr, rcaliman, vsevik, web-inspector-bugs, WebkitBugTracker, webkit.review.bot, yurys|
|Version:||528+ (Nightly build)|
|Bug Depends on:||111969|
Description Johannes Wilm 2012-12-18 16:29:57 PST
If any change to any region is done (using contenteditable=true), a webkitregionlayoutupdate event is triggered for all flows, no matter whether anything was actually changed in any of the regions belonging to other flows than the one that was just edited. I have attached a self-explanatory test file
Comment 1 Johannes Wilm 2012-12-18 16:31:08 PST
Created attachment 180054 [details] a file showing how the webkitregionlayoutupdate event is triggered for all flows, even though only one flow has been changed
Comment 3 Andrei Bucur 2013-03-07 10:10:13 PST
(In reply to comment #1) > Created an attachment (id=180054) [details] > a file showing how the webkitregionlayoutupdate event is triggered for all flows, even though only one flow has been changed This patch fixes most of the unnecessary webkitregionlayoutupdate event dispatches for autoheight regions. It may not be complete though considering how the event is currently defined (triggered every time a flow thread is laid out). Would your use cases be covered if the event changes behavior to be dispatched only when: - an existing region changes the overset property - a region is added to the chain - a region is removed from the chain ?
Comment 5 Johannes Wilm 2013-03-07 10:58:56 PST
@Andrei: At Sourcefabric, our use cases would be covered with the cases you mention. I think this would cover the spec as well, right?
Comment 6 Johannes Wilm 2013-03-07 13:07:15 PST
We would still like an event when an element moves from one region to another, but that should be an event quite apart from this one.
Comment 7 Andrei Bucur 2013-03-19 06:20:38 PDT
Comment on attachment 192030 [details] Patch Unfortunately this patch doesn't treat correctly the complex dependencies between regions. The event as defined right now cannot be implemented easily to dispatch only on the flows that change. See the attached complex example.
Comment 8 Johannes Wilm 2013-03-19 06:27:22 PDT
so this means it's unfixable? Or the CSS specification has to be changed?
Comment 9 Andrei Bucur 2013-03-19 06:27:25 PDT
Created attachment 193808 [details] Complex example Complex example of dependencies between flow threads. When adding the text to the last black border div, the red region with percentage height changes, the red flow is invalidated and all the red regions reflowed. However, the first red region with percentage (near the top blue region) depends on the blue region which is not invalidated. Because of this, the partial layout doesn't give the same results as the full layout obtained when resizing the window: in a full layout the red flow is computed as if the blue region has a height of 0 (as specified). If we invalidate the blue region during the partial layout we will also dispatch regionlayoutupdate for the blue thread as well defeating the initial purpose of the fix.
Comment 10 Andrei Bucur 2013-03-19 07:07:14 PDT
(In reply to comment #8) > so this means it's unfixable? Or the CSS specification has to be changed? It means it's preferable to break the "regionlayoutupdate" event in two events like "regionoversetchanged" and "regionrangeschanged". This split needs to be backed by the CSS specification so we're discussing with the editors about the proposal. The "regionoversetchanged" event would be dispatched as we discussed earlier. The "regionrangeschanged" event would be dispatched when the content inside a region changes (basically when block fragments change).
Comment 12 Mihai Balan 2013-07-15 09:53:58 PDT
The regionlayoutupdate event still has the bug, but not the regionoversetchange. To be closed once the regionlayoutupdate event is removed.
Comment 13 Csaba Osztrogonác 2014-02-13 04:06:01 PST
Comment on attachment 192030 [details] Patch Cleared David Hyatt's review+ from obsolete attachment 192030 [details] so that this bug does not appear in http://webkit.org/pending-commit.