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

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>