Bug 105366 – [CSS Regions] webkitregionlayoutupdate is triggered for all flows even though only one flow has to be updated

• Status: RESOLVED
• Resolution: INVALID
• Component: Web Inspector (Deprecated)

[Johannes Wilm]

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

[Johannes Wilm]

Created attachment 180054 [details]
a file showing how the webkitregionlayoutupdate event is triggered for all flows, even though only one flow has been changed

[Andrei Bucur]

Created attachment 192030 [details]
Patch

[Andrei Bucur]

(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
?

[Dave Hyatt]

Comment on attachment 192030 [details]
Patch

r=me

[Johannes Wilm]

@Andrei: At Sourcefabric, our use cases would be covered with the cases you mention. I think this would cover the spec as well, right?

[Johannes Wilm]

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.

[Andrei Bucur]

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.

[Johannes Wilm]

so this means it's unfixable? Or the CSS specification has to be changed?

[Andrei Bucur]

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.

[Andrei Bucur]

(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).

[Michelangelo De Simone]

Still repros on today's nightly (r151543)

[Mihai Balan]

The regionlayoutupdate event still has the bug, but not the regionoversetchange.

To be closed once the regionlayoutupdate event is removed.

[Csaba Osztrogonác]

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.

[Mihnea Ovidenie]

As regionlayoutupdate was removed in https://bugs.webkit.org/show_bug.cgi?id=132564, closing this bug as invalid.