Bug 146871 – Web Inspector: Inspector should be able to be docked to the bottom of a narrow window

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 256642 [details]
[Animated GIF] Actual/expected

Steps:
1. Make Safari window as narrow as possible, 504px.
2. Open Inspector

Expected:
Inspector opens docked to the bottom of the window if it was docked there previously.
If it opens in a separate window, "dock to bottom" button should be available.

Actual:
Inspector opens in a separate window, dock to bottom button isn't visible.

Note, it is currently still possible to have Inspector docked to the bottom of a narrow (504px) window:
– Dock inspector to a wide window.
– Resize the window to be 504px in width.

At this case, the search field is clipped. We could hide the download button via a CSS media query to make it look nice, see the attached image.

[Radar WebKit Bug Importer]

<rdar://problem/21779296>

[Nikita Vasilyev]

Does the download button do the same as "File > Save As..."? I never use it.

[Timothy Hatcher]

(In reply to comment #2)
> Does the download button do the same as "File > Save As..."? I never use it.

Yes. It is mostly for iOS inspection, where there is no other way to get a web archive. It is fine to hide when needed.

[Nikita Vasilyev]

Do you think this would be a good change?

I have found the following in Protocol/InspectorFrontendAPI.js:

    setDockingUnavailable: function(unavailable)
    {
        WebInspector.updateDockingAvailability(!unavailable);
    },

I couldn't find what calls setDockingUnavailable.

[Timothy Hatcher]

(In reply to comment #4)
> Do you think this would be a good change?
> 
> I have found the following in Protocol/InspectorFrontendAPI.js:
> 
>     setDockingUnavailable: function(unavailable)
>     {
>         WebInspector.updateDockingAvailability(!unavailable);
>     },
> 
> I couldn't find what calls setDockingUnavailable.

Docking is unavailable always on iOS so keep that in mind. The InspectorFrontendAPI functions are called by C++ code in WebKit.

[Nikita Vasilyev]

I have found this:

https://github.com/WebKit/webkit/blob/master/Source/WebCore/inspector/InspectorFrontendClientLocal.cpp#L66

    static const float minimumAttachedWidth = 750.0f;


Changed to 500.0f, compiling...

[Nikita Vasilyev]

(In reply to comment #6)
> I have found this:
> 
> https://github.com/WebKit/webkit/blob/master/Source/WebCore/inspector/
> InspectorFrontendClientLocal.cpp#L66
> 
>     static const float minimumAttachedWidth = 750.0f;
> 
> 
> Changed to 500.0f, compiling...

That didn't change anything.

[Timothy Hatcher]

(In reply to comment #7)
> (In reply to comment #6)
> > I have found this:
> > 
> > https://github.com/WebKit/webkit/blob/master/Source/WebCore/inspector/
> > InspectorFrontendClientLocal.cpp#L66
> > 
> >     static const float minimumAttachedWidth = 750.0f;
> > 
> > 
> > Changed to 500.0f, compiling...
> 
> That didn't change anything.

That value isn't used anymore, maybe only in WebKit1 apps.

There is a similar minimumAttachedWidth in WebKit2 in WebInspector.cpp and in WebInspectorProxyMac.mm. Try those.

[Nikita Vasilyev]

Created attachment 256652 [details]
Patch

[WebKit Commit Bot]

Attachment 256652 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nikita Vasilyev]

(In reply to comment #10)
> Attachment 256652 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and
> either add and list tests, or explain why no new tests were possible. 
> [changelog/nonewtests] [5]
> Total errors found: 1 in 7 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Should I just delete "No new tests (OOPS!)."? I'm not sure what kind of test I should have for this.

[Timothy Hatcher]

Yes, just delete it.

[Nikita Vasilyev]

Created attachment 256656 [details]
Patch

[Timothy Hatcher]

Comment on attachment 256656 [details]
Patch

You should change minimumWindowWidth to be 500 too, which is also in the Internal WebInspector project for iOS. Otherwise, looks good.

[Nikita Vasilyev]

Created attachment 256674 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 256674 [details]
Patch

Clearing flags on attachment: 256674

Committed r186724: <http://trac.webkit.org/changeset/186724>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.