[Webkit-unassigned] [Bug 33096] [Qt] DRT: Support evaluateInWebInspector(), setTimerProfilingEnabled() and display().

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 14 04:52:36 PST 2010


--- Comment #17 from Kent Hansen <kent.hansen at nokia.com>  2010-01-14 04:52:26 PST ---
(In reply to comment #15)
> Created an attachment (id=46511)
 --> (https://bugs.webkit.org/attachment.cgi?id=46511) [details]
> Updated Patch
> I think this is an improvement - only call closewebinspector() when necessary.
> Also clean up some whitespace.

Cool, looking better!
I'm not a reviewer but here are some comments:
- The link to bugzilla should go right below the summary, that way I don't have
to scan the rest of the changelog for it.
- Spelling error in changelog (setTimerProfiilingEnabled).
- In general I'd like to see the inspector-related part of the patch split from
the rest (display(), setTimelineProfilingEnabled()), if that makes sense.
Unless they are tightly related.
- I don't think LayoutController::hideWebInspector() should be removed, it
should rather be renamed to closeWebInspector() (looks like it was just given
the wrong name in the original impl).
- qt_drt_webinspector_execute_script --> qt_drt_webinspector_executeScript
(following the naming convention of other qt_drt functions, e.g.
- closeWebInspector() should set the DeveloperExtrasEnabled attribute to false
after calling close(), as per the Mac and GTK implementations.
- Should closeWebInspector() really check m_webInspector? Since
showWebInspector() bypasses the QWebInspector API, it doesn't look symmetric.
- Rather than doing the URL acrobatics in DumpRenderTree::open(), I recommend
having a shouldOpenWebInspector() helper function (see Mac and GTK port), much
more readable.

Looking forward to being able to run those inspector tests!

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list