Bug 57002

Summary: Web Inspector: source frame should show the error to user when live edit is failed
Product: WebKit Reporter: Pavel Podivilov <podivilov>
Component: Web Inspector (Deprecated)Assignee: Andrey Adaikin <aandrey>
Status: RESOLVED FIXED    
Severity: Normal CC: aandrey, apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 57245    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch yurys: review+

Pavel Podivilov
Reported 2011-03-24 03:52:35 PDT
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.
Attachments
Patch (3.71 KB, patch)
2011-03-25 07:32 PDT, Andrey Adaikin 		no flags
DetailsFormatted DiffDiff
Patch (4.14 KB, patch)
2011-03-25 08:16 PDT, Andrey Adaikin 		yurys: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Andrey Adaikin
Comment 1 2011-03-24 03:56:34 PDT
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
Comment 2 2011-03-25 07:32:21 PDT
Created attachment 86934 [details] Patch
Pavel Podivilov
Comment 3 2011-03-25 07:51:56 PDT
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 4 2011-03-25 08:15:37 PDT
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
Comment 5 2011-03-25 08:16:51 PDT
Created attachment 86945 [details] Patch
Yury Semikhatsky
Comment 6 2011-03-28 01:17:14 PDT
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
Comment 7 2011-03-28 01:56:52 PDT
(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
Comment 8 2011-03-28 02:04:36 PDT
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
Comment 9 2011-03-28 04:32:06 PDT
Committed r82099: <http://trac.webkit.org/changeset/82099>
Note You need to log in before you can comment on or make changes to this bug.