Bug 55360 - Web Inspector: add live edit test
Summary: Web Inspector: add live edit test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-28 02:58 PST by Pavel Podivilov
Modified: 2011-02-28 10:23 PST (History)
10 users (show)

See Also:

Attachments
Patch. (7.56 KB, patch)
2011-02-28 03:05 PST, Pavel Podivilov 		pfeldman: review-
Details | Formatted Diff | Diff
Patch. (10.36 KB, patch)
2011-02-28 06:03 PST, Pavel Podivilov 		pfeldman: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2011-02-28 02:58:33 PST 
Web Inspector: add live edit test.
Comment 1 Pavel Podivilov 2011-02-28 03:05:00 PST 
Created attachment 84036 [details]
Patch.
Comment 2 Pavel Feldman 2011-02-28 03:26:51 PST 
Comment on attachment 84036 [details]
Patch.

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

> LayoutTests/ChangeLog:14
> +        (f):

nuke this line.

> LayoutTests/http/tests/inspector/debugger-test.js:131
> +    InspectorTest.addResult("==Source frame contents start==");

I think there already is a highlighter test that is dumping text model content. Should we add editor-test.js with utility there?

> LayoutTests/inspector/debugger/live-edit.html:21
> +                sourceFrame._textViewer.editLine = InspectorTest.safeWrap(function(element, callback)

You should not override text viewer methods - what if the bug is in that line?
Comment 3 Pavel Podivilov 2011-02-28 06:03:41 PST 
Created attachment 84047 [details]
Patch.
Comment 4 Pavel Podivilov 2011-02-28 06:04:46 PST 
(In reply to comment #2)
> (From update of attachment 84036 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84036&action=review
> 
> > LayoutTests/ChangeLog:14
> > +        (f):
> 
> nuke this line.
Done.
> 
> > LayoutTests/http/tests/inspector/debugger-test.js:131
> > +    InspectorTest.addResult("==Source frame contents start==");
> 
> I think there already is a highlighter test that is dumping text model content. Should we add editor-test.js with utility there?

This method doesn't dump text model, it just dumps the contents. I think we shouldn't reuse highlighter test method here.

> 
> > LayoutTests/inspector/debugger/live-edit.html:21
> > +                sourceFrame._textViewer.editLine = InspectorTest.safeWrap(function(element, callback)
> 
> You should not override text viewer methods - what if the bug is in that line?
Done.
Comment 5 Pavel Podivilov 2011-02-28 10:23:32 PST 
Committed r79880: <http://trac.webkit.org/changeset/79880>