Bug 146871 - Web Inspector: Inspector should be able to be docked to the bottom of a narrow window
Summary: Web Inspector: Inspector should be able to be docked to the bottom of a narro...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-10 20:47 PDT by Nikita Vasilyev
Modified: 2015-07-11 21:24 PDT (History)
8 users (show)

See Also:


Attachments
[Animated GIF] Actual/expected (45.31 KB, image/gif)
2015-07-10 20:47 PDT, Nikita Vasilyev
no flags Details
Patch (4.40 KB, patch)
2015-07-11 01:38 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (4.37 KB, patch)
2015-07-11 10:15 PDT, Nikita Vasilyev
timothy: review-
Details | Formatted Diff | Diff
Patch (6.16 KB, patch)
2015-07-11 19:41 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2015-07-10 20:47:47 PDT
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.
Comment 1 Radar WebKit Bug Importer 2015-07-10 20:48:10 PDT
<rdar://problem/21779296>
Comment 2 Nikita Vasilyev 2015-07-10 20:50:15 PDT
Does the download button do the same as "File > Save As..."? I never use it.
Comment 3 Timothy Hatcher 2015-07-10 20:54:52 PDT
(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.
Comment 4 Nikita Vasilyev 2015-07-10 21:12:28 PDT
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.
Comment 5 Timothy Hatcher 2015-07-10 21:26:51 PDT
(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.
Comment 6 Nikita Vasilyev 2015-07-10 21:40:43 PDT
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...
Comment 7 Nikita Vasilyev 2015-07-10 22:53:09 PDT
(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.
Comment 8 Timothy Hatcher 2015-07-11 00:54:36 PDT
(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.
Comment 9 Nikita Vasilyev 2015-07-11 01:38:49 PDT
Created attachment 256652 [details]
Patch
Comment 10 WebKit Commit Bot 2015-07-11 01:41:34 PDT
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.
Comment 11 Nikita Vasilyev 2015-07-11 01:44:04 PDT
(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.
Comment 12 Timothy Hatcher 2015-07-11 10:08:07 PDT
Yes, just delete it.
Comment 13 Nikita Vasilyev 2015-07-11 10:15:40 PDT
Created attachment 256656 [details]
Patch
Comment 14 Timothy Hatcher 2015-07-11 14:38:26 PDT
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.
Comment 15 Nikita Vasilyev 2015-07-11 19:41:13 PDT
Created attachment 256674 [details]
Patch
Comment 16 WebKit Commit Bot 2015-07-11 21:24:30 PDT
Comment on attachment 256674 [details]
Patch

Clearing flags on attachment: 256674

Committed r186724: <http://trac.webkit.org/changeset/186724>
Comment 17 WebKit Commit Bot 2015-07-11 21:24:35 PDT
All reviewed patches have been landed.  Closing bug.