| Summary: | Web Inspector: Restore regionLayoutUpdated event in iOS7 inspector and remove regionOversetChanged | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Radu Stavila <stavila> | ||||||||||||
| Component: | Web Inspector | Assignee: | Radu Stavila <stavila> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | commit-queue, graouts, joepeck, timothy, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | AdobeTracked, InRadar | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 57312 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Radu Stavila
2014-05-09 01:40:51 PDT
Created attachment 231140 [details]
Patch
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. 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): ... Created attachment 231280 [details]
Patch integrating reviewer feedback
Created attachment 231281 [details]
Patch integrating reviewer feedback
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. 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. Created attachment 231367 [details]
Patch for landing
Created attachment 231369 [details]
Retrying
Comment on attachment 231369 [details] Retrying Clearing flags on attachment: 231369 Committed r168678: <http://trac.webkit.org/changeset/168678> |