RESOLVED FIXED Bug 157714
Web Inspector: breakpoints in sourceURL named scripts are not persisted
https://bugs.webkit.org/show_bug.cgi?id=157714
Summary Web Inspector: breakpoints in sourceURL named scripts are not persisted
Blaze Burg
Reported 2016-05-14 14:00:12 PDT
This makes it very difficult to debug our injected scripts using breakpoints. I don't think this is a regression. STEPS TO REPRODUCE: Using an engineering build, open the debugger tab. Select the __WebInspectorInjectedScript__ resource. Set a breakpoint anywhere. Reload. EXPECTED: The breakpoint should be restored on reload ACTUAL: The breakpoint is not restored
Attachments
Proposed Fix (20.96 KB, patch)
2016-05-14 22:01 PDT, Blaze Burg
joepeck: review+
bburg: commit-queue-
Radar WebKit Bug Importer
Comment 1 2016-05-14 14:00:56 PDT
Blaze Burg
Comment 2 2016-05-14 22:01:22 PDT
Created attachment 278960 [details] Proposed Fix Still need to run tests and such, but it works for injected and normal scripts in my basic testing.
Blaze Burg
Comment 3 2016-05-16 11:34:52 PDT
Comment on attachment 278960 [details] Proposed Fix I did not detect any test regressions caused by this patch, and it seems to work fine locally. Removing 'WIP' from patch, review away!
Joseph Pecoraro
Comment 4 2016-05-16 17:10:49 PDT
Comment on attachment 278960 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=278960&action=review r=me I don't particularly like the name contentIdentifier, but I don't have a better suggestion. > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:38 > + var contentIdentifier = sourceCodeLocationOrInfo.contentIdentifier || sourceCodeLocationOrInfo.url; We should throw a comment in here about sourceCodeLocationOrInfo that the info portion is legacy and can eventually be removed.
Blaze Burg
Comment 5 2016-05-16 21:39:10 PDT
(In reply to comment #4) > Comment on attachment 278960 [details] > Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=278960&action=review > > r=me > > I don't particularly like the name contentIdentifier, but I don't have a > better suggestion. I hear ya. The other straightforward suggestion is contentURL. > > > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:38 > > + var contentIdentifier = sourceCodeLocationOrInfo.contentIdentifier || sourceCodeLocationOrInfo.url; > > We should throw a comment in here about sourceCodeLocationOrInfo that the > info portion is legacy and can eventually be removed.
Blaze Burg
Comment 6 2016-05-17 10:32:24 PDT
Note You need to log in before you can comment on or make changes to this bug.