Bug 161418

Summary: Web Inspector: Can't edit JS resource
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bburg, hi, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch joepeck: review-

Description Nikita Vasilyev 2016-08-30 17:32:36 PDT
Steps:
1. Open http://nv.github.io/webkit-inspector-bugs/debugger-popover-indentation/
2. Open index.js in Debugger or Resources tab.
3. Focus on the content view and type something.

Expected:
Typing works.

Actual:
Can't type.
Comment 1 Radar WebKit Bug Importer 2016-08-30 17:33:03 PDT
<rdar://problem/28087839>
Comment 2 Joseph Pecoraro 2016-08-30 18:27:16 PDT
Users should only be able to edit local (file:///) JavaScript resources. I don't think we've ever been able to edit arbitrary JavaScript resources.
Comment 3 Devin Rousso 2016-08-30 18:56:48 PDT
(In reply to comment #2)
> Users should only be able to edit local (file:///) JavaScript resources. I
> don't think we've ever been able to edit arbitrary JavaScript resources.

That's how I thought it worked ¯\_(ツ)_/¯ (ScriptContentView allows editing with "file" scheme URLs and TextResourceContentView allows editing with CSS files or "file" scheme URLS);

I know that TextEditor defines its CodeMirror instance with "readOnly: true", and SourceCodeTextEditor doesn't seem to change that.
Comment 4 Nikita Vasilyev 2016-08-30 19:07:58 PDT
(In reply to comment #2)
> Users should only be able to edit local (file:///) JavaScript resources. I
> don't think we've ever been able to edit arbitrary JavaScript resources.

I think we should change that. Most decently sized websites aren't developed using file scheme. Of the top of my head, there are two common use cases:

1. Local development server running on http://localhost:1234 or http://myapp.dev.
2. Remote development server with locally mounted partition (via SSHFS).
Comment 5 Devin Rousso 2016-08-30 22:07:10 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > Users should only be able to edit local (file:///) JavaScript resources. I
> > don't think we've ever been able to edit arbitrary JavaScript resources.
> 
> I think we should change that. Most decently sized websites aren't developed
> using file scheme. Of the top of my head, there are two common use cases:
> 
> 1. Local development server running on http://localhost:1234 or
> http://myapp.dev.
> 2. Remote development server with locally mounted partition (via SSHFS).

I agree that local development could benefit from this, but what would editing the resource do (will the new code written ever be executable)?  How about editing a non-local resource?  How do we even distinguish whether the resource is local?
Comment 6 Nikita Vasilyev 2016-08-30 22:46:21 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #2)
> > > Users should only be able to edit local (file:///) JavaScript resources. I
> > > don't think we've ever been able to edit arbitrary JavaScript resources.
> > 
> > I think we should change that. Most decently sized websites aren't developed
> > using file scheme. Of the top of my head, there are two common use cases:
> > 
> > 1. Local development server running on http://localhost:1234 or
> > http://myapp.dev.
> > 2. Remote development server with locally mounted partition (via SSHFS).
> 
> I agree that local development could benefit from this, but what would
> editing the resource do (will the new code written ever be executable)?
> How about editing a non-local resource?

Exactly the same thing as for local resource — just save it.
We currently don't re-execute JavaScript when saving.

> How do we even distinguish whether the
> resource is local?

I don't think it's possible. I want Command-S to always show a dialog to save a file, that's all.
Comment 7 Devin Rousso 2016-08-30 23:17:44 PDT
Created attachment 287491 [details]
Patch
Comment 8 Nikita Vasilyev 2016-08-31 15:44:46 PDT
(In reply to comment #7)
> Created attachment 287491 [details]
> Patch

I just realized that if we're making non-local JS editable, we should do the same for HTML, or any text resource, too. I don't think we should have anything special about the file protocol.

There are issues with managing edited HTML and JS, but they aren't specific to the file protocol.

For example:

    foo.js:
    1. window.addEventListener("click", function() {
    2.     console.log("click")
    3. })

If we set a breakpoint on line 2 and then delete lines 1-3, this breakpoint will be still active and it will trigger on click. This is, however, a separate issue and it has nothing to do with the file protocol.
Comment 9 BJ Burg 2016-09-01 13:01:40 PDT
I am not convinced there is any point allowing to edit the JS if we aren't going to re-execute it or save it to a local filesystem.
Comment 10 Nikita Vasilyev 2016-09-01 14:01:21 PDT
(In reply to comment #9)
> I am not convinced there is any point allowing to edit the JS if we aren't
> going to re-execute it or save it to a local filesystem.

Please read my point in comment #4:

> ...
> Most decently sized websites aren't developed
> using file scheme. Of the top of my head, there are two common use cases:
> 
> 1. Local development server running on http://localhost:1234 or
> http://myapp.dev.
> 2. Remote development server with locally mounted partition (via SSHFS).

In these two cases, it makes sense to allow editing JS.

The use case is:
1. Make some JS changes in Web Inspector on http://myapp.dev.
2. Reload the page.
3. See http://myapp.dev with updated JS.
Comment 11 Joseph Pecoraro 2016-09-01 14:17:28 PDT
Comment on attachment 287491 [details]
Patch

We should discuss and decide an editing policy that makes the most sense. I don't think arbitrarily editing JavaScript/HTML that never gets run on a page, and is lost when the page navigates / reloads, is the right approach.

Based on comments so far, a far more useful and less confusing reason to make resources editable would be if we included the ability to then USE the Web Inspector modified contents to override the next network request for the resource. Then scenarios like editing + reloading could work, and debugging scenarios where you just want to test a quick fix ("what if I changed this HTML / JS Function").

In any case, this needs a more thought out policy.
Comment 12 Matt Baker 2016-10-04 17:19:16 PDT
(In reply to comment #11)
> Comment on attachment 287491 [details]
> Patch
> 
> We should discuss and decide an editing policy that makes the most sense. I
> don't think arbitrarily editing JavaScript/HTML that never gets run on a
> page, and is lost when the page navigates / reloads, is the right approach.
> 
> Based on comments so far, a far more useful and less confusing reason to
> make resources editable would be if we included the ability to then USE the
> Web Inspector modified contents to override the next network request for the
> resource. Then scenarios like editing + reloading could work, and debugging
> scenarios where you just want to test a quick fix ("what if I changed this
> HTML / JS Function").
> 
> In any case, this needs a more thought out policy.

Closing for now. This needs further consideration.