Bug 28124 - Web Inspector: Make all status bar button images glyph-based.
Summary: Web Inspector: Make all status bar button images glyph-based.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-09 05:21 PDT by Pavel Feldman
Modified: 2009-08-12 22:23 PDT (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-08-09 05:21:39 PDT
Currently all the images are rendered together with the WebKit's background. Split background and glyphs for better flexibility.
Comment 1 Pavel Feldman 2009-08-09 05:24:19 PDT
Created attachment 34408 [details]
button-mask-sample
Comment 2 Pavel Feldman 2009-08-09 05:28:23 PDT
Created attachment 34409 [details]
glyphs for buttons extracted from original image source
Comment 3 Pavel Feldman 2009-08-09 05:31:42 PDT
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.
Comment 4 Pavel Feldman 2009-08-09 05:34:36 PDT
Created attachment 34413 [details]
patch (git binary diff format)
Comment 5 Pavel Feldman 2009-08-09 05:40:27 PDT
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.
Comment 6 Timothy Hatcher 2009-08-09 15:35:31 PDT
(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.
Comment 7 Timothy Hatcher 2009-08-09 15:40:38 PDT
(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…
Comment 8 Timothy Hatcher 2009-08-09 15:44:14 PDT
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.
Comment 9 Pavel Feldman 2009-08-09 21:24:33 PDT
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.
Comment 10 Pavel Feldman 2009-08-10 09:55:11 PDT
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 Joseph Pecoraro 2009-08-12 21:53:36 PDT
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 Pavel Feldman 2009-08-12 22:06:48 PDT
(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 Joseph Pecoraro 2009-08-12 22:17:40 PDT
(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 Pavel Feldman 2009-08-12 22:23:12 PDT
(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!