Bug 144329 – [GTK] Web Inspector: New Images added in r183339, r183338, r183332, r183323, r182660, and r182186

• Component: Web Inspector
• Status: RESOLVED FIXED

[Manuel Rego Casasnovas]

Several new images has been added in http://trac.webkit.org/changeset/183323

Specially annoying the missing icon for "Show Details Sidebar" (Images/ToggleRightSidebar.svg) which is the place to change the style of an element.

[Radar WebKit Bug Importer]

<rdar://problem/20726829>

[Andres Gomez Garcia]

Taking ...

[Timothy Hatcher]

More images were added in r183339, r183338, r183332, r183323, r182660, and r182186.

[Andres Gomez Garcia]

Created attachment 252069 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 252069 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252069&action=review

rs=me, but consider the comments to simplify even more

> Source/WebInspectorUI/UserInterface/Images/gtk/Console.svg:1
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>

I had never heard of standalone. Reading up on it:
http://www.xmlplease.com/xml/xmlquotations/standalone#s1.

Says:
Only if the XML document has a DTD, the "standalone" pseudo-attribute could be of importance.
If the XML declaration has no "standalone" pseudo-attribute it defaults to standalone="no".
It is never necessary to use standalone="no" explicitly.

You should be able to remove this attribute from all of these SVGs.

> Source/WebInspectorUI/UserInterface/Images/gtk/Console.svg:5
> +  <path style="block-progression:tb;color:#bebebe" d="m2.1875 0.5c-1.2055 0-2.1875 1.0167-2.1875 2.2188v10.562c0 1.202 0.98197 2.219 2.1875 2.219h11.625c1.206 0 2.188-1.017 2.188-2.219v-10.562c0-1.2023-0.982-2.219-2.188-2.219h-11.625zm0 2h11.625c0.123 0 0.188 0.0809 0.188 0.2188v10.562c0 0.138-0.065 0.219-0.188 0.219h-11.625c-0.1223 0-0.187-0.081-0.187-0.219v-10.562c0-0.1381 0.0647-0.219 0.1875-0.219z"/>

I had never heard of block-progression. It seems to deal with text layout / writing modes. I don't think it is needed here.

You should be able to remove this attribute in all cases that don't apply to text.

> Source/WebInspectorUI/UserInterface/Images/gtk/NewTabPlus.svg:5
> +  <text style="word-spacing:0px;letter-spacing:0px" font-weight="bold" xml:space="preserve" transform="scale(.99917 1.0008)" line-height="125%" font-size="18.288px" y="15.371576" x="0.45220301" font-family="Cantarell" fill="#000000"><tspan y="15.371576" x="0.45220301" fill="#000000">+</tspan></text>

Now here is a case where you are using text! That said, I wonder if it might be more appropriate to use a couple pathes, instead of text. What if the font differs between two GTK machines?

[Andres Gomez Garcia]

(In reply to comment #5)
> > Source/WebInspectorUI/UserInterface/Images/gtk/Console.svg:1
> > +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> 
> I had never heard of standalone. Reading up on it:
> http://www.xmlplease.com/xml/xmlquotations/standalone#s1.
> 
> Says:
> Only if the XML document has a DTD, the "standalone" pseudo-attribute could
> be of importance.
> If the XML declaration has no "standalone" pseudo-attribute it defaults to
> standalone="no".
> It is never necessary to use standalone="no" explicitly.
> 
> You should be able to remove this attribute from all of these SVGs.

Removing ...

> > Source/WebInspectorUI/UserInterface/Images/gtk/Console.svg:5
> > +  <path style="block-progression:tb;color:#bebebe" d="m2.1875 0.5c-1.2055 0-2.1875 1.0167-2.1875 2.2188v10.562c0 1.202 0.98197 2.219 2.1875 2.219h11.625c1.206 0 2.188-1.017 2.188-2.219v-10.562c0-1.2023-0.982-2.219-2.188-2.219h-11.625zm0 2h11.625c0.123 0 0.188 0.0809 0.188 0.2188v10.562c0 0.138-0.065 0.219-0.188 0.219h-11.625c-0.1223 0-0.187-0.081-0.187-0.219v-10.562c0-0.1381 0.0647-0.219 0.1875-0.219z"/>
> 
> I had never heard of block-progression. It seems to deal with text layout /
> writing modes. I don't think it is needed here.
> 
> You should be able to remove this attribute in all cases that don't apply to
> text.

Removing all the unneeded text related attributes ...

> > Source/WebInspectorUI/UserInterface/Images/gtk/NewTabPlus.svg:5
> > +  <text style="word-spacing:0px;letter-spacing:0px" font-weight="bold" xml:space="preserve" transform="scale(.99917 1.0008)" line-height="125%" font-size="18.288px" y="15.371576" x="0.45220301" font-family="Cantarell" fill="#000000"><tspan y="15.371576" x="0.45220301" fill="#000000">+</tspan></text>
> 
> Now here is a case where you are using text! That said, I wonder if it might
> be more appropriate to use a couple pathes, instead of text. What if the
> font differs between two GTK machines?

This is clearly a mistake that slipped among my fingers :(

Obviously, everything should have been converted to shapes. Thanks to this, now I've found another file that uses also a font (the one from which a derived this one). I will fix that in the optimizing patch that I'm already cooking.

Thanks a lot for your review Joseph (Joe?) :) !

[Andres Gomez Garcia]

Created attachment 252383 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 252383 [details]
Patch for landing

Clearing flags on attachment: 252383

Committed r183806: <http://trac.webkit.org/changeset/183806>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

> Thanks a lot for your review Joseph (Joe?) :) !

Joe, JoePeck, Joseph, all work for me =)