Bug 57002 – Web Inspector: source frame should show the error to user when live edit is failed

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Podivilov]

Web Inspector: source frame should show the error to user when live edit is failed.

Currently, the only way to check whether live edit failed or succeeded is to open the console and check for warnings.
Source frame should indicate live edit failure more explicitly.

[Andrey Adaikin]

One way is just to open the console on an error, and leave the editor in the live edit mode, preserving the current (unsaved) changes.

Any other ideas?

[Andrey Adaikin]

Created attachment 86934 [details]
Patch

[Pavel Podivilov]

Comment on attachment 86934 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86934&action=review

> Source/WebCore/inspector/front-end/ScriptsPanel.js:1064
>                  WebInspector.log(newBodyOrErrorMessage, WebInspector.ConsoleMessage.MessageLevel.Warning);

I think you should either move this code to SourceFrame or move WebInspector.showConsole() here.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:1065
> +            callback(success);

When live edit is succeeded, newBodyOrErrorMessage contains new source which may differ from current SourceFrame content.

[Andrey Adaikin]

Comment on attachment 86934 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86934&action=review

>> Source/WebCore/inspector/front-end/ScriptsPanel.js:1064
>>                  WebInspector.log(newBodyOrErrorMessage, WebInspector.ConsoleMessage.MessageLevel.Warning);
> 
> I think you should either move this code to SourceFrame or move WebInspector.showConsole() here.

done.

>> Source/WebCore/inspector/front-end/ScriptsPanel.js:1065
>> +            callback(success);
> 
> When live edit is succeeded, newBodyOrErrorMessage contains new source which may differ from current SourceFrame content.

done.

[Andrey Adaikin]

Created attachment 86945 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 86945 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86945&action=review

> Source/WebCore/inspector/front-end/SourceFrame.js:817
> +                if (newSource !== newBodyOrErrorMessage)

Is there a valid scenario for this?

[Pavel Podivilov]

(In reply to comment #6)
> (From update of attachment 86945 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=86945&action=review
> 
> > Source/WebCore/inspector/front-end/SourceFrame.js:817
> > +                if (newSource !== newBodyOrErrorMessage)
> 
> Is there a valid scenario for this?

There is no such scenario according to Peter.

[Andrey Adaikin]

Applying the diff below before landing:

diff --git a/Source/WebCore/inspector/front-end/SourceFrame.js b/Source/WebCore/inspector/front-end/SourceFrame.js
index de8b9fb..6f5d0dc 100644
--- a/Source/WebCore/inspector/front-end/SourceFrame.js
+++ b/Source/WebCore/inspector/front-end/SourceFrame.js
@@ -811,12 +811,7 @@ WebInspector.SourceFrame.prototype = {
 
         function didEditScriptSource(success, newBodyOrErrorMessage)
         {
-            if (this._originalTextModelContent !== undefined || this._textModel.text !== newSource)
-                return;
-            if (success) {
-                if (newSource !== newBodyOrErrorMessage)
-                    this._textModel.setText(null, newBodyOrErrorMessage);
-            } else {
+            if (!success && this._originalTextModelContent === undefined && this._textModel.text === newSource) {
                 this._originalTextModelContent = originalTextModelContent;
                 this._textViewer.readOnly = false;
                 this._delegate.setScriptSourceIsBeingEdited(true);

[Pavel Podivilov]

Committed r82099: <http://trac.webkit.org/changeset/82099>