Bug 105366 - [CSS Regions] webkitregionlayoutupdate is triggered for all flows even though only one flow has to be updated
Summary: [CSS Regions] webkitregionlayoutupdate is triggered for all flows even though...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrei Bucur
URL:
Keywords: AdobeTracked
Depends on: 111969
Blocks: 57312
  Show dependency treegraph
 
Reported: 2012-12-18 16:29 PST by Johannes Wilm
Modified: 2014-05-08 08:23 PDT (History)
17 users (show)

See Also:


Attachments
a file showing how the webkitregionlayoutupdate event is triggered for all flows, even though only one flow has been changed (1.24 KB, text/html)
2012-12-18 16:31 PST, Johannes Wilm
no flags Details
Patch (16.24 KB, patch)
2013-03-07 10:03 PST, Andrei Bucur
no flags Details | Formatted Diff | Diff
Complex example (3.70 KB, text/html)
2013-03-19 06:27 PDT, Andrei Bucur
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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 2 Andrei Bucur 2013-03-07 10:03:21 PST
Created attachment 192030 [details]
Patch
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 4 Dave Hyatt 2013-03-07 10:52:59 PST
Comment on attachment 192030 [details]
Patch

r=me
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 11 Michelangelo De Simone 2013-06-13 11:50:22 PDT
Still repros on today's nightly (r151543)
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.
Comment 14 Mihnea Ovidenie 2014-05-08 08:23:52 PDT
As regionlayoutupdate was removed in https://bugs.webkit.org/show_bug.cgi?id=132564, closing this bug as invalid.