Bug 203722 - Web Inspector: REGRESSION(r249831): content view is empty if a breakpoint is hit in the main resource
Summary: Web Inspector: REGRESSION(r249831): content view is empty if a breakpoint is ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 201535
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-31 21:26 PDT by Devin Rousso
Modified: 2019-11-01 12:17 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.06 KB, patch)
2019-10-31 21:30 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (2.65 KB, patch)
2019-11-01 00:29 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-10-31 21:26:12 PDT
# STEPS TO REPRODUCE:
1. inspect any page with an inline <script>
2. set a breakpoint in that <script>
3. reload
 => pause information is shown, but the content view is empty
 => continuing doesn't update the content view
Comment 1 Devin Rousso 2019-10-31 21:26:24 PDT
<rdar://problem/56802409>
Comment 2 Devin Rousso 2019-10-31 21:30:36 PDT
Created attachment 382549 [details]
Patch
Comment 3 Devin Rousso 2019-11-01 00:29:11 PDT
Created attachment 382565 [details]
Patch

This gets part of the way there, in that the content is now visible.

But there appears to be another issue, which is that the initial content is now just the inline script content, rather than the entire HTML document.  This seems to have been caused by r249831.  My guess as to why that's happening is that the timing of showing the pause location within the inline script is now not working correctly since we have to wait for the formatted HTML content, which is asynchronous.  I think we should either not auto-format HTML resources if we're showing a pause location the first time the resource is shown or try to delay the highlighting code until after the format is complete.
Comment 4 Joseph Pecoraro 2019-11-01 10:38:05 PDT
Comment on attachment 382565 [details]
Patch

rs=me
Comment 5 Joseph Pecoraro 2019-11-01 10:40:22 PDT
(In reply to Devin Rousso from comment #3)
> Created attachment 382565 [details]
> Patch
> 
> This gets part of the way there, in that the content is now visible.
> 
> But there appears to be another issue, which is that the initial content is
> now just the inline script content,

I think that might have been expected behavior.

If I recall correctly, the frontend would have paused in a Script but not necessarily the main resource content... as backwards as that sounds. We've had situations in the past where we would show blank content and just a <script>...</script> if you pause in script during page load.

That said, if we have the full content, we should address it!
Comment 6 Devin Rousso 2019-11-01 10:48:25 PDT
(In reply to Joseph Pecoraro from comment #5)
> (In reply to Devin Rousso from comment #3)
> > Created attachment 382565 [details]
> > Patch
> > 
> > This gets part of the way there, in that the content is now visible.
> > 
> > But there appears to be another issue, which is that the initial content is now just the inline script content,
> 
> I think that might have been expected behavior.
> 
> If I recall correctly, the frontend would have paused in a Script but not necessarily the main resource content... as backwards as that sounds. We've had situations in the past where we would show blank content and just a <script>...</script> if you pause in script during page load.
> 
> That said, if we have the full content, we should address it!
Before r249831 though, we did show the *full* content.  This is 100% a regression.

I'm pretty sure we also have the full content by this point, so we should definitely show it.
Comment 7 Devin Rousso 2019-11-01 11:01:10 PDT
Comment on attachment 382565 [details]
Patch

I'm going to land this as is since it's already a positive change.

I've created a followup for the "missing the rest of the content" issue:
 - <https://webkit.org/b/203748> Web Inspector: REGRESSION(r249831): hitting a breakpoint in an inline <script> doesn't show the surrounding HTML content
Comment 8 WebKit Commit Bot 2019-11-01 12:17:18 PDT
Comment on attachment 382565 [details]
Patch

Clearing flags on attachment: 382565

Committed r251932: <https://trac.webkit.org/changeset/251932>
Comment 9 WebKit Commit Bot 2019-11-01 12:17:19 PDT
All reviewed patches have been landed.  Closing bug.