Bug 132731

Summary: Web Inspector: Restore regionLayoutUpdated event in iOS7 inspector and remove regionOversetChanged
Product: WebKit Reporter: Radu Stavila <stavila>
Component: Web InspectorAssignee: 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 Flags
Patch
timothy: review-
Patch integrating reviewer feedback
none
Patch integrating reviewer feedback
joepeck: review+
Patch for landing
none
Retrying none

Radu Stavila
Reported 2014-05-09 01:40:51 PDT
Web Inspector: Restore regionLayoutUpdated event in iOS7 inspector and remove regionOversetChanged
Attachments
Patch (6.75 KB, patch)
2014-05-09 04:06 PDT, Radu Stavila 		timothy: review-
DetailsFormatted DiffDiff
Patch integrating reviewer feedback (5.27 KB, patch)
2014-05-12 02:27 PDT, Radu Stavila 		no flags
DetailsFormatted DiffDiff
Patch integrating reviewer feedback (5.26 KB, patch)
2014-05-12 02:28 PDT, Radu Stavila 		joepeck: review+
DetailsFormatted DiffDiff
Patch for landing (5.26 KB, patch)
2014-05-13 04:04 PDT, Radu Stavila 		no flags
DetailsFormatted DiffDiff
Retrying (5.26 KB, patch)
2014-05-13 04:51 PDT, Radu Stavila 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-05-09 01:42:56 PDT
<rdar://problem/16864929>
Radu Stavila
Comment 2 2014-05-09 04:06:15 PDT
Created attachment 231140 [details] Patch
Timothy Hatcher
Comment 3 2014-05-09 09:20:47 PDT
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 4 2014-05-09 14:03:00 PDT
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
Comment 5 2014-05-12 02:27:27 PDT
Created attachment 231280 [details] Patch integrating reviewer feedback
Radu Stavila
Comment 6 2014-05-12 02:28:53 PDT
Created attachment 231281 [details] Patch integrating reviewer feedback
Radu Stavila
Comment 7 2014-05-12 02:31:59 PDT
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 8 2014-05-12 10:16:42 PDT
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
Comment 9 2014-05-13 04:04:56 PDT
Created attachment 231367 [details] Patch for landing
Radu Stavila
Comment 10 2014-05-13 04:51:15 PDT
Created attachment 231369 [details] Retrying
WebKit Commit Bot
Comment 11 2014-05-13 06:11:55 PDT
Comment on attachment 231369 [details] Retrying Clearing flags on attachment: 231369 Committed r168678: <http://trac.webkit.org/changeset/168678>
Note You need to log in before you can comment on or make changes to this bug.