Bug 115996 - Save sheet on the Web Inspector does not come out from under the toolbar
Summary: Save sheet on the Web Inspector does not come out from under the toolbar
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-05-12 17:09 PDT by Timothy Hatcher
Modified: 2013-05-13 14:54 PDT (History)
8 users (show)

See Also:


Attachments
Patch (20.83 KB, patch)
2013-05-12 17:18 PDT, Timothy Hatcher
joepeck: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2013-05-12 17:09:52 PDT
The save sheet comes out where the toolbar is expected to be if the labels are showing (1px off still). In other toolbar modes the sheet floats in the middle of the window.

This is based on NSWindow's contentBorderThickness. We should be updating contentBorderThickness anyway to get the right window shadow under the different height toolbars. This just makes it obvious we are not setting this for the different modes.

<rdar://problem/13871067>
Comment 1 Timothy Hatcher 2013-05-12 17:18:16 PDT
Created attachment 201513 [details]
Patch
Comment 2 Joseph Pecoraro 2013-05-13 11:28:30 PDT
Comment on attachment 201513 [details]
Patch

Looks good to me. Can we get an owner to r+ the WK2 part (very simple).
Comment 3 Alexey Proskuryakov 2013-05-13 11:34:27 PDT
Comment on attachment 201513 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        (InspectorFrontendClient):

It's best to remove such erroneous prepare-ChangeLog output, as it doesn't help people who are reading ChangeLogs. One day, someone should fix prepare-ChangeLog.
Comment 4 Benjamin Poulain 2013-05-13 11:36:11 PDT
Comment on attachment 201513 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.h:89
> +    virtual void setToolbarHeight(unsigned);

OVERRIDE

> Source/WebKit/gtk/WebCoreSupport/InspectorClientGtk.h:96
> +    virtual void setToolbarHeight(unsigned);

OVERRIDE

> Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h:113
> +    virtual void setToolbarHeight(unsigned);

OVERRIDE

> Source/WebKit/qt/WebCoreSupport/InspectorClientQt.h:99
> +    virtual void setToolbarHeight(unsigned);

OVERRIDE

> Source/WebKit/win/WebCoreSupport/WebInspectorClient.h:112
> +    virtual void setToolbarHeight(unsigned);

OVERRIDE

> Source/WebKit2/UIProcess/WebInspectorProxy.h:130
> +#if PLATFORM(MAC)
> +    void setToolbarHeight(unsigned height) { platformSetToolbarHeight(height); }
> +#else
> +    void setToolbarHeight(unsigned) { }
> +#endif
> +

Instead of this, you should have both setToolbarHeight and platformSetToolbarHeight defined for both platforms and platformSetToolbarHeight() implemented as 
{
    notImplemented();
}
Comment 5 Benjamin Poulain 2013-05-13 11:36:40 PDT
I sign off on this with the comments above.
Comment 6 Timothy Hatcher 2013-05-13 14:54:14 PDT
r150041 (and a change to the Safari Inspector)