Bug 132731 – Web Inspector: Restore regionLayoutUpdated event in iOS7 inspector and remove regionOversetChanged

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Radu Stavila]

Web Inspector: Restore regionLayoutUpdated event in iOS7 inspector and remove regionOversetChanged

[Radar WebKit Bug Importer]

<rdar://problem/16864929>

[Radu Stavila]

Created attachment 231140 [details]
Patch

[Timothy Hatcher]

Comment on attachment 231140 [details]
Patch

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

Looking good. We can reuse the new path internally. The observer change and event name map are really the only changes needed.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:61
> +    RegionLayoutUpdated: "dom-tree-manager-region-layout-updated",  /* COMPATIBILITY (iOS 7 and Safari 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

Only iOS has compatibility concerns. Safari on OS X always has a matching WebKit.

We can reuse the new event internally. The only compatibility layer needed is in the Observer file.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:620
> +    /* COMPATIBILITY (iOS 7 and Safari 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

Ditto.

> Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:193
> +        nameMap.set("webkitregionlayoutupdate", "Region Layout Update");    /* COMPATIBILITY (iOS 7 and Safari 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

This is needed.

> Source/WebInspectorUI/UserInterface/Protocol/CSSObserver.js:67
> +    regionLayoutUpdated: function(namedFlow)

And this is needed. But it can just regionOversetChanged.

[Timothy Hatcher]

Comment on attachment 231140 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Protocol/CSSObserver.js:66
> +    /* COMPATIBILITY (iOS 7 and Safari 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

Should just be: // COMPATIBILITY (iOS 7): ...

[Radu Stavila]

Created attachment 231280 [details]
Patch integrating reviewer feedback

[Radu Stavila]

Created attachment 231281 [details]
Patch integrating reviewer feedback

[Radu Stavila]

Comment on attachment 231281 [details]
Patch integrating reviewer feedback

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

> Source/WebInspectorUI/UserInterface/Protocol/Legacy/7.0/InspectorWebBackendCommands.js:206
> +InspectorBackend.registerEvent("CSS.regionLayoutUpdated", ["namedFlow"]);

I also left these 7.0 legacy rollbacks in because I made these changes in https://bugs.webkit.org/show_bug.cgi?id=132564 and https://bugs.webkit.org/show_bug.cgi?id=132566 and I now realise they were wrong. Please let me know if it's ok.

> Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json:-2188
> -                "name": "regionOversetChanged",

Ditto.

[Joseph Pecoraro]

Comment on attachment 231281 [details]
Patch integrating reviewer feedback

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

r=me

> Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:193
> +        nameMap.set("webkitregionlayoutupdate", "Region Layout Update");    /* COMPATIBILITY (iOS 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

Style: This should be a single line comment, and should end in a period. We try not to use multiline comments.

> Source/WebInspectorUI/UserInterface/Protocol/CSSObserver.js:66
> +    /* COMPATIBILITY (iOS 7): regionLayoutUpdated was removed and replaced by regionOversetChanged */

Style: This should be a single line comment, and should end in a period. We try not to use multiline comments.

[Radu Stavila]

Created attachment 231367 [details]
Patch for landing

[Radu Stavila]

Created attachment 231369 [details]
Retrying

[WebKit Commit Bot]

Comment on attachment 231369 [details]
Retrying

Clearing flags on attachment: 231369

Committed r168678: <http://trac.webkit.org/changeset/168678>