1421051 - urlbar-tab.svg still has the old tab style

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement, P3)

Description

[Johann Hofmann [:johannh]]

The "Switch to tab" icon in the URL bar still has the rounded corners of old:

https://searchfox.org/mozilla-central/source/browser/themes/shared/urlbar-tab.svg

We should probably replace it with something new.

Comment 1

[Amin Al Hazwani [:amin] (Firefox UX)]

We have a new icon :) You can find it here http://design.firefox.com/icons/viewer/#tab!

Comment 2

[Johann Hofmann [:johannh]]

Sounds like a good first bug then.

Comment 3

[Amin Al Hazwani [:amin] (Firefox UX)]

Also, jfyi the new icon is currently being used in the "Synced Tabs" view in the sidebar.

Comment 4

[Vedant]

I'd like to take up this bug if no one has taken it yet

Comment 5

[Constantin Pavelescu]

Attachment: changed tab icon with the new one in the comments above me

Hello, 

I changed the icon with the one that is present in the comments above. 

Please let me know if the patch is good. 

I would appreciate timely feedback as this is part of one university module of mine. 

Thank you, 
Regards, 
Constantin

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8934743 [details] [diff] [review]
changed tab icon with the new one in the comments above me

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

Hey, thanks for taking this bug!

It seems like you were working on a very outdated branch of mozilla-central (the original urlbar-tab.svg in your patch is from before May 24).

Can you try to pull a newer version and make a new commit on top of that?

Let me know if you have any questions!

Comment 7

[Vedant]

Attachment: patch file for bug 1421051

I've created and attached a patch fixing this issue.

Comment 8

[Vedant]

Attachment: introduce new tab icon for "switch to tab"

Comment 9

[Johann Hofmann [:johannh]]

Comment on attachment 8935006 [details] [diff] [review]
patch file for bug 1421051

Hi Vedant,

thank you for your contribution, the patch looks very good. Since Constantin had already started to work on the bug (and is officially assigned to it) I'd like to give him the chance to complete it, especially considering that this seems to be part of a course work. Hence I'm cancelling review on your patch. Please ask the person assigned to a bug before taking over their work in the future.

I'd love if you could take a stab at bug 1422846 instead, which is currently not assigned. If you leave a comment I will happily assign it to you.

Thanks for helping out!

Comment 10

[Vedant]

Apologies Johann,

I did not see that it had already been assigned. And thank you, I will take a look at it.

Comment 11

[Constantin Pavelescu]

Hello, 

Thank you for that! I am surprised to see so much activity and timely responses on here! 

I appreciate that! I will update my codebase and redo the patch shortly. 

Thank you, 
Regards, 
Constantin

Comment 12

[Constantin Pavelescu]

Attachment: Patch using the updated version of the codebase

Hello, 

As it turned out, I was indeed working on a very outdated version of the codebase. 

I updated it now, so please tell me if this patch is all good. 

Thank you, and thank you for keeping me as the assignee of this bug. 

Regards, 
Constantin

Comment 13

[Constantin Pavelescu]

(In reply to Constantin Pavelescu from comment #12)
> Created attachment 8935116 [details] [diff] [review]
> Patch using the updated version of the codebase
> 
> Hello, 
> 
> As it turned out, I was indeed working on a very outdated version of the
> codebase. 
> 
> I updated it now, so please tell me if this patch is all good. 
> 
> Thank you, and thank you for keeping me as the assignee of this bug. 
> 
> Regards, 
> Constantin

Sorry, I just checked it and the patch file seems to be empty. I will upload the good one shortly. Trying to figure out why it wrote an empty patch since I did exactly the same steps as I did before when it wrote a valid patch. 

Sorry for the inconvenience. 
Will be back shortly.

Comment 14

[Constantin Pavelescu]

Attachment: Hopefully it's the good file this time

Hello, 

Sorry for the inconvenience, I still don't know why it didn't do a patch that time. 

Hopefully it's going to be fine now. 

Please assist me with the review. 

Thank you, 
Regards, 
Constantin

Comment 15

[Johann Hofmann [:johannh]]

Comment on attachment 8935123 [details] [diff] [review]
Hopefully it's the good file this time

Sorry for the delay, I was out sick. Unfortunately this is basically the same patch as before, the changes can't be applied to a recent version of central.

Are you using Mercurial or Git? Did you run hg pull central and hg up central before applying your changes? You can find more info here:

http://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html#feature-development

Let me know if you have any questions!

Comment 16

[Johann Hofmann [:johannh]]

Hi Constantin, are you still looking into this?

Comment 17

[Johann Hofmann [:johannh]]

Let's take Vedant's patch then!

Comment 18

[Johann Hofmann [:johannh]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac78853b7fc9fa46249944f7ef1e086cba5a7154
Bug 1421051 - Introduce new tab icon for "switch to tab" r=johannh

Comment 19

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/ac78853b7fc9