Bug 28124 – Web Inspector: Make all status bar button images glyph-based.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

Currently all the images are rendered together with the WebKit's background. Split background and glyphs for better flexibility.

[Pavel Feldman]

Created attachment 34408 [details]
button-mask-sample

[Pavel Feldman]

Created attachment 34409 [details]
glyphs for buttons extracted from original image source

[Pavel Feldman]

Created attachment 34411 [details]
-webkit-mask glitch

When mask is smaller than the glyph div, the border around the mask is transparent and one can see the background. This is only visible when running the frontend as Web Inspector after dock/undock operations, this is not repeatable when opening frontend as a regular web page.

As a workaround, I made all glyphs 32x24.

[Pavel Feldman]

Created attachment 34413 [details]
patch (git binary diff format)

[Pavel Feldman]

Created attachment 34414 [details]
patch (git binary diff format)

Note that I could not make it work as
<button>
    <div class="glyph"></div>
    <div class="glyph shadow"></div>
</button>

since glyph divs were positioned absolutely with respect to button's container (that is a div with display:inline-block). 

I ended up with 

<button>
    <div style="position:relative">
        <div class="glyph"></div>
        <div class="glyph shadow"></div>
    </div>
</button>

and I am open to the hints / suggestions.

[Timothy Hatcher]

(In reply to comment #5)
> Created an attachment (id=34414) [details]
> patch (git binary diff format)
> 
> Note that I could not make it work as
> <button>
>     <div class="glyph"></div>
>     <div class="glyph shadow"></div>
> </button>
> 
> since glyph divs were positioned absolutely with respect to button's container
> (that is a div with display:inline-block). 
> 
> I ended up with 
> 
> <button>
>     <div style="position:relative">
>         <div class="glyph"></div>
>         <div class="glyph shadow"></div>
>     </div>
> </button>
> 
> and I am open to the hints / suggestions.

Can't you just make the button's container position: relative? Thats what I did in my sample.

[Timothy Hatcher]

(In reply to comment #3)
> Created an attachment (id=34411) [details]
> -webkit-mask glitch
> 
> When mask is smaller than the glyph div, the border around the mask is
> transparent and one can see the background. This is only visible when running
> the frontend as Web Inspector after dock/undock operations, this is not
> repeatable when opening frontend as a regular web page.
> 
> As a workaround, I made all glyphs 32x24.

That sounds/looks like a bug with CSS masks that you should file, it looks like the mask is being iverted…

[Timothy Hatcher]

Comment on attachment 34414 [details]
patch (git binary diff format)

I think if you make button.status-bar-item position: relative you wont need the extra container. The CSS file had tbas in it also.

[Pavel Feldman]

Created attachment 34438 [details]
patch (git binary diff format)

>> Can't you just make the button's container position: relative? Thats what I did in my sample.

I missed it, yes, it does work.

>> I think if you make button.status-bar-item position: relative you wont need the
extra container. The CSS file had tbas in it also.

Done.

>> That sounds/looks like a bug with CSS masks that you should file, it looks like
the mask is being iverted…

Once this is submitted, I will create another bug and attach a small glyph to it so that one could reproduce it.
I will store small glyphs in the meanwhile so that we can use them later.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	WebCore/inspector/front-end/Images/clearConsoleButtons.png
	D	WebCore/inspector/front-end/Images/consoleButtons.png
	D	WebCore/inspector/front-end/Images/dockButtons.png
	D	WebCore/inspector/front-end/Images/enableButtons.png
	D	WebCore/inspector/front-end/Images/excludeButtons.png
	D	WebCore/inspector/front-end/Images/focusButtons.png
	D	WebCore/inspector/front-end/Images/largerResourcesButtons.png
	D	WebCore/inspector/front-end/Images/nodeSearchButtons.png
	D	WebCore/inspector/front-end/Images/pauseOnExceptionButtons.png
	D	WebCore/inspector/front-end/Images/percentButtons.png
	D	WebCore/inspector/front-end/Images/recordButtons.png
	D	WebCore/inspector/front-end/Images/reloadButtons.png
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/inspector/front-end/ElementsPanel.js
	A	WebCore/inspector/front-end/Images/clearConsoleButtonGlyph.png
	A	WebCore/inspector/front-end/Images/consoleButtonGlyph.png
	A	WebCore/inspector/front-end/Images/dockButtonGlyph.png
	A	WebCore/inspector/front-end/Images/enableOutlineButtonGlyph.png
	A	WebCore/inspector/front-end/Images/enableSolidButtonGlyph.png
	A	WebCore/inspector/front-end/Images/excludeButtonGlyph.png
	A	WebCore/inspector/front-end/Images/focusButtonGlyph.png
	A	WebCore/inspector/front-end/Images/largerResourcesButtonGlyph.png
	A	WebCore/inspector/front-end/Images/nodeSearchButtonGlyph.png
	A	WebCore/inspector/front-end/Images/pauseOnExceptionButtonGlyph.png
	A	WebCore/inspector/front-end/Images/percentButtonGlyph.png
	A	WebCore/inspector/front-end/Images/recordButtonGlyph.png
	A	WebCore/inspector/front-end/Images/recordToggledButtonGlyph.png
	A	WebCore/inspector/front-end/Images/reloadButtonGlyph.png
	A	WebCore/inspector/front-end/Images/undockButtonGlyph.png
	M	WebCore/inspector/front-end/Panel.js
	M	WebCore/inspector/front-end/ProfilesPanel.js
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
Committed r46979

[Joseph Pecoraro]

This missed one in DOMStorageItemsView.js, namely because its a View and not a Panel:

Approx line 35:

  -    this.deleteButton = document.createElement("button");
  +    this.deleteButton = WebInspector.Panel.prototype.createStatusBarButton();

I'll be needing to apply the same to my CookieItemsView when I add that, should I patch this at the same time?

[Pavel Feldman]

(In reply to comment #11)
> This missed one in DOMStorageItemsView.js, namely because its a View and not a
> Panel:
> 
> Approx line 35:
> 
>   -    this.deleteButton = document.createElement("button");
>   +    this.deleteButton =
> WebInspector.Panel.prototype.createStatusBarButton();
> 
> I'll be needing to apply the same to my CookieItemsView when I add that, should
> I patch this at the same time?

More or less. You should not call methods on prototypes - use appropriate Panel object instead. Note that the status button class is being introduced in https://bugs.webkit.org/show_bug.cgi?id=28174, so you either need to wait until it is landed and use the new way, or patch createStatusBarButton in now and request that 28174 is updated to migrate you to the new way of living!

[Joseph Pecoraro]

(In reply to comment #12)
> More or less. You should not call methods on prototypes - use appropriate Panel
> object instead.

Good advice, I should have thought of that.

> Note that the status button class is being introduced in
> https://bugs.webkit.org/show_bug.cgi?id=28174, so you either need to wait until
> it is landed and use the new way, or patch createStatusBarButton in now and
> request that 28174 is updated to migrate you to the new way of living!

I didn't know about that!  That looks awesome!  I think including it in that patch would be best.  I'll post a comment on that bug.

-----

Also, the refresh button wasn't turned into a glyph. Was there a reason for that?  (They can be found in DOMStorageItemsView.js and the just moments ago new CookiesItemsView.js constructors).

[Pavel Feldman]

(In reply to comment #13)
> (In reply to comment #12)
> > More or less. You should not call methods on prototypes - use appropriate Panel
> > object instead.
> 
> Good advice, I should have thought of that.
> 
> > Note that the status button class is being introduced in
> > https://bugs.webkit.org/show_bug.cgi?id=28174, so you either need to wait until
> > it is landed and use the new way, or patch createStatusBarButton in now and
> > request that 28174 is updated to migrate you to the new way of living!
> 
> I didn't know about that!  That looks awesome!  I think including it in that
> patch would be best.  I'll post a comment on that bug.
> 
> -----
> 
> Also, the refresh button wasn't turned into a glyph. Was there a reason for
> that?  (They can be found in DOMStorageItemsView.js and the just moments ago
> new CookiesItemsView.js constructors).

Oh, I think I missed it since I did not see it in the UI. I'll convert it today and pass to Mikhail so that he could land it with 28174! Thanks for letting me know!