Bug 132564 – [CSS Regions] Remove regionLayoutUpdate event

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Radu Stavila]

The regionLayoutUpdate event is deprecated and should be removed. It was replaced by the regionOversetChange event (http://blogs.adobe.com/webplatform/2013/07/15/css-regions-new-working-draft/)

[Radu Stavila]

Created attachment 230832 [details]
Patch

[Radu Stavila]

Latest CSS Regions WD: http://www.w3.org/TR/css3-regions/

[WebKit Commit Bot]

Comment on attachment 230832 [details]
Patch

Clearing flags on attachment: 230832

Committed r168306: <http://trac.webkit.org/changeset/168306>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 230832 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230832&action=review

To clarify my comments, I only object to a few of the WebInspectorUI changes. The frontend is meant to be backwards compatible with older backends (e.g. iOS 6 and iOS 7). Where possible we should support what the older backends were legitimately capable of doing. Unless something complex changes that would make supporting a legacy case difficult (which I don't believe is the case here), we should strive to maintain backwards compatibility.

Eventually if we decide to stop supporting inspecting iOS 7 we can find the COMPATIBILITY comments and remove the associated code.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:-61
> -    RegionLayoutUpdated: "dom-tree-manager-region-layout-updated",

So this wasn't used?

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:-623
> -    regionLayoutUpdated: function(flowPayload)
> -    {
> -        this._sendNamedFlowUpdateEvents(flowPayload);
> -    },

These should be left in for legacy.

> Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:-193
> -        nameMap.set("webkitregionlayoutupdate", "Region Layout Update");

You could put a COMPATIBILITY comment here.

> Source/WebInspectorUI/UserInterface/Protocol/CSSObserver.js:-69
> -    regionLayoutUpdated: function(namedFlow)
> -    {
> -        WebInspector.domTreeManager.regionLayoutUpdated(namedFlow);
> -    },

Here in the Observer we should add a comment that this is for legacy only. Something like:

    // COMPATIBILITY (iOS 7): regionLayoutUpdated has transitioned to regionOversetChanged.

> Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json:-2192
> -            },
> -            {
> -                "name": "regionLayoutUpdated",
> -                "parameters": [
> -                    { "name": "namedFlow", "$ref": "NamedFlow", "description": "The Named Flow whose layout may have changed." }
> -                ],
> -                "description": "Fires when a Named Flow's layout may have changed."

So technically this still exists in iOS 7 and we should leave it in and handle it for the best debugging experience.

[Radu Stavila]

Created https://bugs.webkit.org/show_bug.cgi?id=132731