Bug 28124 - Web Inspector: Make all status bar button images glyph-based.
: Web Inspector: Make all status bar button images glyph-based.
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-08-09 05:21 PST by
Modified: 2009-08-12 22:23 PST (History)


Attachments
button-mask-sample (3.61 KB, application/octet-stream)
2009-08-09 05:24 PST, Pavel Feldman
no flags Details
glyphs for buttons extracted from original image source (6.64 KB, application/octet-stream)
2009-08-09 05:28 PST, Pavel Feldman
no flags Details
-webkit-mask glitch (130.19 KB, image/png)
2009-08-09 05:31 PST, Pavel Feldman
no flags Details
patch (git binary diff format) (100.41 KB, patch)
2009-08-09 05:34 PST, Pavel Feldman
no flags Review Patch | Details | Formatted Diff | Diff
patch (git binary diff format) (100.41 KB, patch)
2009-08-09 05:40 PST, Pavel Feldman
timothy: review-
Review Patch | Details | Formatted Diff | Diff
patch (git binary diff format) (100.20 KB, patch)
2009-08-09 21:24 PST, Pavel Feldman
timothy: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-08-09 05:21:39 PST
Currently all the images are rendered together with the WebKit's background. Split background and glyphs for better flexibility.
------- Comment #1 From 2009-08-09 05:24:19 PST -------
Created an attachment (id=34408) [details]
button-mask-sample
------- Comment #2 From 2009-08-09 05:28:23 PST -------
Created an attachment (id=34409) [details]
glyphs for buttons extracted from original image source
------- Comment #3 From 2009-08-09 05:31:42 PST -------
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.
------- Comment #4 From 2009-08-09 05:34:36 PST -------
Created an attachment (id=34413) [details]
patch (git binary diff format)
------- Comment #5 From 2009-08-09 05:40:27 PST -------
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.
------- Comment #6 From 2009-08-09 15:35:31 PST -------
(In reply to comment #5)
> Created an attachment (id=34414) [details] [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.
------- Comment #7 From 2009-08-09 15:40:38 PST -------
(In reply to comment #3)
> Created an attachment (id=34411) [details] [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…
------- Comment #8 From 2009-08-09 15:44:14 PST -------
(From update of attachment 34414 [details])
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.
------- Comment #9 From 2009-08-09 21:24:33 PST -------
Created an attachment (id=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.
------- Comment #10 From 2009-08-10 09:55:11 PST -------
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
------- Comment #11 From 2009-08-12 21:53:36 PST -------
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?
------- Comment #12 From 2009-08-12 22:06:48 PST -------
(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!
------- Comment #13 From 2009-08-12 22:17:40 PST -------
(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).
------- Comment #14 From 2009-08-12 22:23:12 PST -------
(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!