1155402 - Standalone client needs new prompt for Chrome 44

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Adam Roach [:abr]]

Attachment: Screenshot 2015-04-14 15.28.04.png

It would appear that we need a new camera prompt for Chrome starting with version 44: they've gone from a ribbon to a doorhanger for camera/mic permissions.

See attached screenshot for an example.

Comment 1

[Adam Roach [:abr]]

Attachment: Chrome2.svg

Attached, find the new prompt graphic provided by Sevaan.

Comment 2

[:shell escalante]

determine when chrome 44 is releasing to make sure we pull it for then.  at 42 currently - not sure on cycles.

Comment 3

[:shell escalante]

chrome 44 is live right now - so need to prioritize this one.

is this a good first bug?

Comment 4

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

Hey I would like to work on this bug please assign it to me :)

I think I figured out the most difficult part of it :) I just need to replace the current gum-chrome.svg with the one attached by you in the comment 2 

at the path mozilla-central/browser/components/loop/standalone/content/img 

only thing that I am left with is to learn how to submit the patch :)

Comment 5

[Bogdan Maris, Desktop Test Engineering]

I`ve added Thummala as assignee of this bug since he requested this on #irc and he does not have the privileges to do so.

Comment 6

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

Attachment: bug-1155402.patch changed the gum-chrome.svg file to the one attached in the bugzilla

changed the gum-chrome.svg file to the one attached in the bugzilla attachment 2 [details] [diff] [review]

Hope you guys review the patch quickly and apply it :)

Comment 7

[Anup Kumar [:reznord]]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #5)
> I`ve added Thummala as assignee of this bug since he requested this on #irc
> and he does not have the privileges to do so.

Hi Bogdan Maris,
Should I go create a PR for tryserver? just tell me what are the tests required.


Hi Dhanvi,
The patch looks good, just a small heads up for you for the next time onwards, 

1) When you include the commit message to the patch, include "r= mentor_name". That is the standard format of a patch. :) 

2) Everytime you create a patch, just give a review to the mentor so that he can verify the patch.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Mike's currently away from work; switching the review to Standard8 who will be back (though probably a bit swamped) on Monday.

Comment 9

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

I have to edit the patch to match the file format of others in the same directory 

Is it necessary @standard8

Comment 10

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

Attachment: bug-1155402-v2

I have submitted the final patch please take

Comment 11

[Mark Banner (:standard8)]

I was a bit worried about needing to cope with older versions of Chrome. However, I took a look at our stats and the majority of the hits we got over the last week or so are chrome 44. Hence I think we can keep it simple and just replace the image directly - rather than having a before & after.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 8641549 [details] [diff] [review]
bug-1155402.patch changed the gum-chrome.svg file to the one attached in the bugzilla

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

We should go with the other patch that has the license with it.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8642502 [details] [diff] [review]
bug-1155402-v2

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

Looks great. Thanks for the patch, there's a couple of minor amendments, but since they are minor I'll fix them for you on check-in which I'll do in a couple of minutes.

> Bug 1155402 - Change standalone prompt for Chrome 44

My suggest would be something like "Bug 1155402 - Change Loop's standalone prompt for gUM to align with Chrome 44 changes.". Its a little bit more descriptive, includes the "Loop" word so that non people can identify it as part of Loop/Hello without looking into the details, and includes a bit more detail as to what we're changing.

::: browser/components/loop/standalone/content/img/gum-chrome.svg
@@ +1,5 @@
>  <?xml version="1.0" encoding="UTF-8" standalone="no"?>
>  <!-- 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 width="101px" height="62px" viewBox="0 0 101 62" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

This svg element still needs the ' xmlns:sketch="http://www.bohemiancoding.com/sketch/ns"' within it otherwise it isn't readable.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e0973a29cb1c

Comment 15

[Mark Banner (:standard8)]

Ok, this has now landed in our integration branch, it'll be merged to the main mozilla-central repository sometime in the next 24 hours or so (at which time this bug will automatically be marked as fixed)

The new image will be live the next time we do a standalone release, which is likely to be in about 2 weeks.

Thanks again for the patch.

Comment 16

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

(In reply to Mark Banner (:standard8) from comment #13)
> Comment on attachment 8642502 [details] [diff] [review]
> bug-1155402-v2
> 
> Review of attachment 8642502 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great. Thanks for the patch, there's a couple of minor amendments, but
> since they are minor I'll fix them for you on check-in which I'll do in a
> couple of minutes.
> 
> > Bug 1155402 - Change standalone prompt for Chrome 44
> 
> My suggest would be something like "Bug 1155402 - Change Loop's standalone
> prompt for gUM to align with Chrome 44 changes.". Its a little bit more
> descriptive, includes the "Loop" word so that non people can identify it as
> part of Loop/Hello without looking into the details, and includes a bit more
> detail as to what we're changing.
I will keep that In mind :) 
> 
> ::: browser/components/loop/standalone/content/img/gum-chrome.svg
> @@ +1,5 @@
> >  <?xml version="1.0" encoding="UTF-8" standalone="no"?>
> >  <!-- 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 width="101px" height="62px" viewBox="0 0 101 62" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
> 
> This svg element still needs the '
> xmlns:sketch="http://www.bohemiancoding.com/sketch/ns"' within it otherwise
> it isn't readable.

I thought It was just some branding as I was able to view the image without the line in my LXDE image viewer

Comment 17

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

(In reply to Mark Banner (:standard8) from comment #15)
> Ok, this has now landed in our integration branch, it'll be merged to the
> main mozilla-central repository sometime in the next 24 hours or so (at
> which time this bug will automatically be marked as fixed)
> 
> The new image will be live the next time we do a standalone release, which
> is likely to be in about 2 weeks.
Thanks for the heads up 
> 
> Thanks again for the patch.

:) cool more patches coming for some other components

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Bogdan Maris, Desktop Test Engineering]

Verified as fixed making a call between Firefox 42beta3 and Chrome 45.0.2454.101 across platforms (Windows 7 64-bit, Mac OS X 10.10.5 and Ubuntu 14.04 32-bit).