Bug 19873 - Inspector should support clear() in the command line
Summary: Inspector should support clear() in the command line
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Keishi Hattori
URL:
Keywords: InRadar
Depends on:
Blocks: 14355
  Show dependency treegraph
 
Reported: 2008-07-03 06:44 PDT by Keishi Hattori
Modified: 2008-08-15 10:06 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (6.07 KB, patch)
2008-08-12 09:25 PDT, Keishi Hattori
timothy: review-
Details | Formatted Diff | Diff
better patch (1.21 KB, patch)
2008-08-14 07:18 PDT, Keishi Hattori
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2008-07-03 06:44:05 PDT
Inspector should support clear() in the command line for Firebug parity.
Comment 1 Adam Roben (:aroben) 2008-07-11 14:07:42 PDT
<rdar://problem/6070221>
Comment 2 Keishi Hattori 2008-08-12 09:25:34 PDT
Created attachment 22750 [details]
proposed patch

I've added console.clear so that I can talk to the Inspector from within the inspected window. Maybe we don't want non-Firebug compliant stuff in "console"?
Comment 3 Timothy Hatcher 2008-08-13 00:23:52 PDT
Comment on attachment 22750 [details]
proposed patch

I think this patch looks fine, no technical issues. But please spend some time on your ChangeLogs. You are missing the Console.cpp and InspectorController.cpp changes. The prepare-ChangeLog script will do most of the work for you.
Comment 4 Kevin McCullough 2008-08-13 11:10:30 PDT
Yes, besides the changelogs this patch looks good.  I think it's cleaner to call console.clear() than call back into InspectorController().  Also this should fix a bug where closing the inspector cleared all messages.
Comment 5 Adam Roben (:aroben) 2008-08-13 11:24:26 PDT
(In reply to comment #4)
> I think it's cleaner to call console.clear() than call back into InspectorController().

But do we really want to allow web pages to clear the Inspector's console? That neither seems like something a web page need to be able to do, nor does it seem good for our users (since a "malicious" web page could clear the console without their permission).
Comment 6 Kevin McCullough 2008-08-13 11:34:29 PDT
Hmmm it seems that clear will remove error badges from the bottom bar and from the resources panel.  I think it would be ok to let pages clear messages if they didn't also clear their errors and warnings about attempts to do cross-site scripting or other malicious attempts.

On the other hand I can see how a developer might know about a set of warnings and errors and want to get them out of their site automatically so they don't clutter the dev's work.
Comment 7 Kevin McCullough 2008-08-13 11:35:08 PDT
What if the console was cleared but the badging wasn't?
Comment 8 Keishi Hattori 2008-08-14 07:18:42 PDT
Created attachment 22790 [details]
better patch

I learned about InspectorController.wrapCallback :-)
Comment 9 Timothy Hatcher 2008-08-14 10:48:37 PDT
Comment on attachment 22790 [details]
better patch

Looks good and it does seem better to not add console.clear now that I think about the issue with it. But I can imagine it being useful.
Comment 10 Timothy Hatcher 2008-08-15 10:06:26 PDT
Landed in r35786.