Bug 79649 - Web Inspector: WebKit2: need API to when the Inspector is in the front
Summary: Web Inspector: WebKit2: need API to when the Inspector is in the front
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: Timothy Hatcher
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-27 02:03 PST by Timothy Hatcher
Modified: 2013-01-04 06:11 PST (History)
13 users (show)

See Also:


Attachments
Proposed Change (14.34 KB, patch)
2012-02-27 02:13 PST, Timothy Hatcher
sullivan: review-
Details | Formatted Diff | Diff
Proposed Change (round 2) (14.40 KB, patch)
2012-02-27 09:10 PST, Timothy Hatcher
sullivan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2012-02-27 02:03:58 PST
Need an API to know when the Inspector window is front or not, not just visible.
Comment 1 Timothy Hatcher 2012-02-27 02:13:39 PST
Created attachment 128990 [details]
Proposed Change
Comment 2 WebKit Review Bot 2012-02-27 02:17:03 PST
Attachment 128990 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 John Sullivan 2012-02-27 06:06:57 PST
Comment on attachment 128990 [details]
Proposed Change

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

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:126
> +        [menuItem setTitle:WEB_UI_STRING("Hide Web Inspector", "title for Hide Web Inspector menu item")];

Don’t you also need to handle the “Show Web Inspector” case here?

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:257
> +    return m_isVisible && [m_inspectorView.get().window isMainWindow];

It would be good to reference a bug number in this FIXME.
Comment 4 Brian Weinstein 2012-02-27 06:52:51 PST
Comment on attachment 128990 [details]
Proposed Change

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

> Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:245
> +    notImplemented();

It looks like you need NotImplemented.h for the Windows build.
Comment 5 Timothy Hatcher 2012-02-27 08:58:12 PST
(In reply to comment #3)
> (From update of attachment 128990 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=128990&action=review
> 
> > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:126
> > +        [menuItem setTitle:WEB_UI_STRING("Hide Web Inspector", "title for Hide Web Inspector menu item")];
> 
> Don’t you also need to handle the “Show Web Inspector” case here?

No, sicne if the Web Inspector's window delegate is handleing the validation we know it is front. I'll add a comment.

> > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:257
> > +    return m_isVisible && [m_inspectorView.get().window isMainWindow];
> 
> It would be good to reference a bug number in this FIXME.

Will do.

I'll fix those and the Windows build and upload a new patch.
Comment 6 Timothy Hatcher 2012-02-27 09:10:56 PST
Created attachment 129049 [details]
Proposed Change (round 2)
Comment 7 WebKit Review Bot 2012-02-27 09:13:25 PST
Attachment 129049 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 John Sullivan 2012-02-27 09:15:47 PST
Comment on attachment 129049 [details]
Proposed Change (round 2)

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

> Source/WebCore/ChangeLog:8
> +

Needs a bug number here.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:95
> +}

Is there something preventing us from updating the method name to showOrHideWebInspector: or something along those lines?

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:254
> +    // FIXME: this will not bring a background tab in Safari to the front, only its window. <rdar://problem/10937688>

Our normal style is "FIXME 10937688:" or "FIXME <rdar://problem/10937688>".

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:260
> +    // FIXME: this will not return false for a background tab in Safari, only a background window. <rdar://problem/10937688>

Same here.
Comment 9 Eric Seidel (no email) 2013-01-04 00:53:44 PST
Attachment 129049 [details] was posted by a committer and has review+, assigning to Timothy Hatcher for commit.
Comment 10 Timothy Hatcher 2013-01-04 06:11:27 PST
This landed in r109002.