1392585 - Fix busted build due to dupes: chrome/toolkit/skin/classic/global/icons/find-next-arrow.svg chrome/classic/skin/classic/messenger/icons/arrow-dropdown.svg

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

ERROR: The following duplicated files are not allowed:
chrome/toolkit/skin/classic/global/icons/find-next-arrow.svg
chrome/classic/skin/classic/messenger/icons/arrow-dropdown.svg

Looks like M-C landed find-next-arrow.svg and it's identical to our arrow-dropdown.svg.

Let me land a bustage fix now and we'll look at it later.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1392585-dupes.patch

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/29da01df42f2
Add find-next-arrow.svg and arrow-dropdown.svg to allowed duplicates. rs=bustage-fix

Comment 3

[Jorg K (CEST = GMT+2)]

M-C changed their arrow and used ours:
https://hg.mozilla.org/mozilla-central/rev/ff913409f74c#l22.1

Both files are now identical:

<!-- This Source Code Form is subject to the terms of the Mozilla Public
   - License, v. 2.0. If a copy of the MPL was not distributed with this
   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <path fill="context-fill" d="M8 12a1 1 0 0 1-.707-.293l-5-5a1 1 0 0 1 1.414-1.414L8 9.586l4.293-4.293a1 1 0 0 1 1.414 1.414l-5 5A1 1 0 0 1 8 12z"/>
</svg>

I let Richard decide whether to live with the duplicates or start using their arrow which was ours before.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: Bug1392585.patch

I checked the icons. We could use the toolkit icon but it's name is not so well speaking (find-next-arrow.svg) how it looks. FX uses the toolkit arrow-dropdown-16.svg. But this file is not yet a Photon image, the stroke is too thin, like the find-next-arrow.svg. It can be they will do this later. Then we can decide again after this is done.

I optimized our SVG and it is now shorter and different to the toolkit icon but looks still the same. I don't think they will optimize their icon the same way as :ntim normally does this at the beginning before check-in. So we should be save.

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8899918 [details] [diff] [review]
Bug1392585.patch

Review of attachment 8899918 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, good idea. How do you optimise? I can see that you rounded some numbers, but I don't understand the change at the end.

::: mail/themes/shared/mail/icons/arrow-dropdown.svg
@@ +1,5 @@
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16">
> +  <path fill="context-fill" d="M8 12a1 1 0 0 1-.7-.3l-5-5a1 1 0 0 1 1.4-1.4L8 9.6l4.3-4.3a1 1 0 0 1 1.4 1.4l-5 5a1 1 0 0 1-.7.3z"/>

What's the "1-.7.3z" at the end which was "1 8 12z"?

Comment 6

[Richard Marti (:Paenglab)]

I'm using https://jakearchibald.github.io/svgomg/ to optimize. You could also have SVGO as a command line tool locally.

(In reply to Jorg K (GMT+2) from comment #5)
> What's the "1-.7.3z" at the end which was "1 8 12z"?

The difference is some chars before this change, the "5A1" to "5a1". A upper case command, the "A" means absolute positions, and a lower case "a" means relative positions. Here the end point is now relative positioned to the last points coordinate before the "a". It wasn't me who decided to use relative positions, it was SVGO. :)

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8899918 [details] [diff] [review]
Bug1392585.patch

Any objections to adding |width="16" height="16"| back in since it makes a difference when viewed in IE.

Comment 8

[Richard Marti (:Paenglab)]

Also in FX it makes a difference. But it's not needed because we define the dimensions already in CSS https://dxr.mozilla.org/comm-central/source/mail/themes/shared/mail/tabmail.css#256

And this could also be a difference after the toolkit image would be optimized.

Comment 9

[Jorg K (CEST = GMT+2)]

All the other SVGs I checked have width and height. I'd rather have it for consistency. I'll deal with the next bustage due to this.

Comment 10

[Richard Marti (:Paenglab)]

Attachment: Bug1392585.patch

Okay, no problem. :)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/89f6848383e3
Optimize our arrow-dropdown.svg to not be a duplicate of find-next-arrow.svg. r=jorgk DONTBUILD