702284 - Additional descriptive text for Help > About $PRODUCTNAME in mobile Aurora and Nightly

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Lawrence Mandel [:lmandel] (use needinfo)]

The following text should be added to the About boxes in both Aurora and Nightly, in preparation for the upcoming Telemetry-by-default in these testing releases.

$PRODUCTNAME is experimental software and things break from time to time. $PRODUCTNAME automatically sends useful test information back to Mozilla so that we can make Firefox better.

Note that this is the same description as Tom provided in bug 701182.

Comment 1

[:Margaret Leibovic]

Right now there isn't an about box/place in Fennec Native (at least not one I can find). Is there a bug/plan to add that?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Margaret Leibovic [:margaret] from comment #1)
> Right now there isn't an about box/place in Fennec Native (at least not one
> I can find). Is there a bug/plan to add that?

Could it be added to about:fennec?

Comment 3

[:Margaret Leibovic]

(In reply to Jared Wein [:jwein and :jaws] from comment #2)
> (In reply to Margaret Leibovic [:margaret] from comment #1)
> > Right now there isn't an about box/place in Fennec Native (at least not one
> > I can find). Is there a bug/plan to add that?
> 
> Could it be added to about:fennec?

Ah, yeah. I didn't know that existed. Thanks for pointing that out!

Comment 4

[Lawrence Mandel [:lmandel] (use needinfo)]

The final text that we went with in desktop (bug 701182) is:

$PRODUCTNAME is experimental and may be unstable. It automatically sends test information back to Mozilla to help make $PRODUCTNAME better.

Can we add this text to the top of about:fennec for Nightly and Aurora?

Comment 5

[Théo Chevalier]

Attachment: Patch V1

A first patch, need feedback.

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 583594 [details] [diff] [review]
Patch V1

Theo - Glad to see you pick this one up.

Looks good to me. We want this in both native for phones and xul for tablets. Do you have a screenshot of how this looks on a phone and on a tablet?

Comment 7

[Théo Chevalier]

(In reply to Lawrence Mandel [:lmandel] from comment #6)
> Comment on attachment 583594 [details] [diff] [review]
> Patch V1
> 
> Theo - Glad to see you pick this one up.
> 
> Looks good to me. We want this in both native for phones and xul for
> tablets. Do you have a screenshot of how this looks on a phone and on a
> tablet?

Thank you! Unfortunately, I can not compile Fennec yet(I still have to configure everything on my laptop...). I think I will request access to try servers, it will be more convenient for the future. Maybe you could help me for that?

Comment 8

[Théo Chevalier]

Hum, seems that I just can't compile it on Windows: https://wiki.mozilla.org/Mobile/Fennec/Android_OtherBuildEnvs
I'll really need a try server acces :(

Comment 9

[:Margaret Leibovic]

(In reply to Théo Chevalier from comment #8)
> Hum, seems that I just can't compile it on Windows:
> https://wiki.mozilla.org/Mobile/Fennec/Android_OtherBuildEnvs
> I'll really need a try server acces :(

Théo, you should apply for level 1 commit access. Follow the instructions here, and if you cc me to the bug I can vouch for you:

http://www.mozilla.org/hacking/committer/

Comment 10

[Théo Chevalier]

Thanks Margaret! I filed a bug for it:
https://bugzilla.mozilla.org/show_bug.cgi?id=712798

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Madhava - Is adding text to the about:fennec page something that UX wants to review?

Comment 12

[Théo Chevalier]

Attachment: XUL UI (Screenshot)

I finally compile my own Fennec build, here are the screenshots.

Comment 13

[Théo Chevalier]

Attachment: Tablet version V1

Comment 14

[:Margaret Leibovic]

(In reply to Théo Chevalier [:tchevalier] from comment #12)
> Created attachment 585520 [details]
> Phone version V1
> 
> I finally compile my own Fennec build, here are the screenshots.

This looks like XUL Fennec. Théo, can you build native Fennec? We should be targeting that, since we're on schedule to release that on the Firefox 11 train. I'm mentioning this because the theme of about:fennec recently changed, so we should make sure it still looks okay.

(If you have trouble building, you can usually find help on IRC in #mobile)

Comment 15

[Théo Chevalier]

(In reply to Margaret Leibovic [:margaret] from comment #14)
> (In reply to Théo Chevalier [:tchevalier] from comment #12)
> > Created attachment 585520 [details]
> > Phone version V1
> > 
> > I finally compile my own Fennec build, here are the screenshots.
> 
> This looks like XUL Fennec. Théo, can you build native Fennec? We should be
> targeting that, since we're on schedule to release that on the Firefox 11
> train. I'm mentioning this because the theme of about:fennec recently
> changed, so we should make sure it still looks okay.
> 
> (If you have trouble building, you can usually find help on IRC in #mobile)

Oh, yes you're right, we have new native UI, I was thinking that UI is changing like a media query with screen width. I'll build native Fennec. Just downloading Android SDK, NDK, and a virtual Android.

Comment 16

[Lawrence Mandel [:lmandel] (use needinfo)]

Theo - Thanks for you continued work on this bug. Have you had success building the native UI? Do you need any help moving forward?

Comment 17

[Théo Chevalier]

(In reply to Lawrence Mandel [:lmandel] from comment #16)
> Theo - Thanks for you continued work on this bug. Have you had success
> building the native UI? Do you need any help moving forward?

Need some serious help. I was going to ask on IRC what was causing this: http://www.uploadup.com/di-XSTT.jpg

I build my own native build on Linux with help of Margaret, loaded it on my virtual Android 4.0.3 on Win 7 x64, and all the pages (except the about:home get this black content. I tried disabling webGL (Maybe used for Layers?) with ac_add_options --disable-webgl, but it didn't helped. I'm out of ideas...

Comment 18

[Lawrence Mandel [:lmandel] (use needinfo)]

Theo - I understand that Mark reached out to you to offer some assistance. Have you been able to get past your hurdle?

Comment 19

[Théo Chevalier]

(In reply to Lawrence Mandel [:lmandel] from comment #18)
> Theo - I understand that Mark reached out to you to offer some assistance.
> Have you been able to get past your hurdle?

Not yet, unfortunately.  Mark told me to add
 sHasDirectTexture = false;
in http://mxr.mozilla.org/mozilla-central/source/widget/android/nsWindow.cpp#872

I've recompile Fennec, but it didn't help: still that black screen. I'll try to see with him, or somebody else, if there is another solution.
 This build is going to completely ruin myself. :(

Comment 20

[Théo Chevalier]

bug 718647 - Fennec displays black content in emulator.

Maybe we've to find another way to get a screenshot... If somebody have an Android device and he can install this build (http://www.theochevalier.fr/files/fennec-12.0a1.en-US.android-arm.apk), and post a screenshot of about:fennec here, it will be very appreciate.

Comment 21

[Lawrence Mandel [:lmandel] (use needinfo)]

Attachment: about:firefox screenshot with Theo's build

Here you go Theo.

Comment 22

[Lawrence Mandel [:lmandel] (use needinfo)]

@madhava - See the screenshot that I attached in comment 21. The text that Theo is adding needs some love (so that I can actually read it). Do you have a suggestion on how to display the text? Below the Firefox logo? On top of the logo?

Comment 23

[Théo Chevalier]

(In reply to Lawrence Mandel [:lmandel] from comment #21)
> Created attachment 590799 [details]
> about:firefox screenshot with Theo's build
> 
> Here you go Theo.

Great, thanks! So... yes, it needs some love.

My personal POV, is to put the text below the Firefox logo, with a smaller font, and the same padding as the links below it. But, it's only my POV, let Madhava give his own.

Comment 24

[Madhava Enros [:madhava]]

Hi guys - Ian has bugs/designs for the nightly and aurora-specific versions of the about:fennec pages. I would suggest working the text into those designs.

CC-ing Ian.

Comment 25

[Ian Barlow (:ibarlow)]

Attachment: mockup

Hey guys, here's a mockup of how we could improve the legibility of that disclaimer a bit. We can place it under the "Check for Updates" button, centered, and in the same font size as the version number. 

You'll probably also notice that this design uses a special Aurora-specific theme. This is based on work that's happening in bug 71260.

Hope this helps, let me know if you have any questions about the design!

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

Thanks Ian. That works for me. I'm happy to get the text in and let UX iterate on positioning. Do you have a mockup for Nightly as well?

Comment 27

[Théo Chevalier]

(In reply to Ian Barlow (:ibarlow) from comment #25)
> Created attachment 590829 [details]
> mockup
> 
> Hey guys, here's a mockup of how we could improve the legibility of that
> disclaimer a bit. We can place it under the "Check for Updates" button,
> centered, and in the same font size as the version number. 
> 
> You'll probably also notice that this design uses a special Aurora-specific
> theme. This is based on work that's happening in bug 71260.
> 
> Hope this helps, let me know if you have any questions about the design!

Thanks Ian and Madhava!

I'll reproduce this asap!
I guess the bug number was 710260 :)

Comment 28

[Théo Chevalier]

Attachment: Patch V2

I think this patch is an improvement of the last, but there is still some work needed to match the mockup...

Lawrence - I'll post within 2 hours a build, could you, please, test the build on your terminal and take a screenshot?
If nothing appear, I'll post another build who "force" the text to be displayed, and then I'll fix that. But if the text appears with the first build, it's cool :) Thanks!

Comment 29

[Théo Chevalier]

Build with Patch V2: http://www.theochevalier.fr/files/patch2-build1/fennec-12.0a1.en-US.android-arm.apk

This build is still crashing at startup on my virtual android, with this logcat: https://bug718647.bugzilla.mozilla.org/attachment.cgi?id=591986

Lawrence - (Or someone else) If you have time to test it on your phone, it would be great :)

Comment 30

[Théo Chevalier]

This first build is crashing at startup on an Android 2.3.4 device.

Comment 31

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Théo Chevalier [:tchevalier] from comment #30)
> This first build is crashing at startup on an Android 2.3.4 device.

As you said, the build crashes as soon as the app launches. I'm on a Samsung Galaxy S II.

Comment 32

[Théo Chevalier]

(In reply to Lawrence Mandel [:lmandel] from comment #31)
> (In reply to Théo Chevalier [:tchevalier] from comment #30)
> > This first build is crashing at startup on an Android 2.3.4 device.
> 
> As you said, the build crashes as soon as the app launches. I'm on a Samsung
> Galaxy S II.

Yeah, thanks Lawrence. I think this is caused by bug 707320, and he has just been backed out, so, I'll retry.

Comment 33

[Théo Chevalier]

Attachment: Screenshot Patch V2

Need to center it and remove the "float". I'll post a new patch. 

+ I'll change the message into:
It automatically sends information about performance, hardware, usage and customizations back to &vendorShortName; to help make &brandShortName; better.
Accordingly bug 728350.

Comment 34

[Théo Chevalier]

Attachment: PatchV3

Comment 35

[Théo Chevalier]

(Wrong bug, sorry...)

Comment 36

[Théo Chevalier]

Attachment: Patch V3

I will post a screenshot of this tomorrow.

Comment 37

[Théo Chevalier]

Attachment: Screenshot native UI

PatchV3 without #ifdef MOZ_TELEMETRY_REPORTING (Works only on Firefox desktop)

I'll reduce the bottom margin, and remove the 1em padding to match the mockup.
And obviously, find a solution for the top margin.

Comment 38

[Théo Chevalier]

Attachment: Patch V4

I compensated the 1em padding in updateBox by setting the width to 70% instead of 60% to match as close to the mockup.
I'll provide a screenshot of this tomorrow.

Final version of the text is being discussed in bug 728340.

Comment 39

[Théo Chevalier]

Attachment: Native UI (screenshot)

The black border is not in the patch, it's just to see the position of the div relative to the update button.

Comment 40

[Théo Chevalier]

Attachment: Patch V5

Improvement: Underline the Mozilla link, because we can't see this is a link.

Comment 41

[Ian Barlow (:ibarlow)]

Comment on attachment 598662 [details]
Native UI (screenshot)

This looks great, Théo, thanks!

Comment 42

[Lawrence Mandel [:lmandel] (use needinfo)]

Looks great Théo. Can you update the text to match the new desktop text from bug 728350?

"It automatically sends information about performance, hardware, usage and customizations back to &vendorShortName; to help make &brandShortName; better."

Comment 43

[Théo Chevalier]

Attachment: Patch V6

Comment 44

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 599635 [details] [diff] [review]
Patch V6

>diff --git a/mobile/android/chrome/content/about.xhtml b/mobile/android/chrome/content/about.xhtml
>+  <div id="aboutTelemetry" hidden="true">
>+      <p id="telemetry">&aboutPage.telemetryStart;<a href="http://www.mozilla.org/">&aboutPage.telemetryMozillaLink;</a>&aboutPage.telemetryEnd;</p>
>+    </div>

r+, but submit a new patch with the indent fixed for "</div>"

Comment 45

[Théo Chevalier]

Attachment: Patch V6 (indent update)

Oops, corrected!

Comment 46

[Théo Chevalier]

If someone could land it for me, it'll be much appreciated :)

We can land it like that, but after, in Bug 737596 I'll display the message regarding MOZ_TELEMETRY_ON_BY_DEFAULT, so it will be displayed only on Nightly and Aurora, where telemetry is enabled by default.

Comment 47

[Théo Chevalier]

Ho, just thinking about it, if I want to hide the telemetry part, I've to split the text right after "&brandShortName; is experimental and may be unstable.", to display that part in all the cases, and only hide the telemetry related part.
I'll do it tomorrow. :)

Comment 48

[Théo Chevalier]

Attachment: Patch V7

Ok now it's good.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/77123b152b5c

I assume you'll be requesting approval-aurora too.

Comment 50

[Théo Chevalier]

(In reply to Ryan VanderMeulen from comment #49)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/77123b152b5c
> 
> I assume you'll be requesting approval-aurora too.

Thanks for the push :)
Yeah, good idea, like that we could enable Telemetry by default in Aurora at the same time.

Comment 51

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/77123b152b5c

Comment 52

[Francesco Lodolo [:flod]]

Is there a specific reason for that starting blank space in aboutPage.telemetryStart?

Comment 53

[Axel Hecht [:Pike]]

So, on desktop, the unstable warning only gets enabled for nightly and aurora, but the telemetry note is shown always. Thus the split of strings.

But here, we hide the complete telementry note including the stability warning.

Is that intended?

Also, if it wasn't, the space should probably go into the hidden part, aka, the warning instead of the note.

Comment 54

[Théo Chevalier]

(In reply to Axel Hecht [:Pike] from comment #53)
> So, on desktop, the unstable warning only gets enabled for nightly and
> aurora, but the telemetry note is shown always. Thus the split of strings.
> 
> But here, we hide the complete telementry note including the stability
> warning.
> 
> Is that intended?

Yes, it will be modified when telemetry will be enabled by default. We'll only have a #ifdef between the version warning and the telemetry warning. The telemetry part will be only show on "nightly" and "aurora" channels where TELEMETRY_ON_BY_DEFAULT is defined.

Comment 55

[Bill Gianopoulos [:WG9s]]

So besides seemingly causing bug 745567, looking at the patches, the one that landed seems to be marked as having been reviewed by the submitter???

Comment 56

[Bill Gianopoulos [:WG9s]]

Patch author identified as  Théo Chevalier [:tchevalier] reviewer identified as theo.chevalier11: review+ what kind of review process is that?

Comment 57

[Théo Chevalier]

(In reply to Bill Gianopoulos [:WG9s] from comment #56)
> Patch author identified as  Théo Chevalier [:tchevalier] reviewer identified
> as theo.chevalier11: review+ what kind of review process is that?

What's wrong? I've done what mfinkle told me to do on IRC. The changes where minors, he said I can use the r+ he gave me for the previous patch, and it was needed to mark the patch as r+ to identify the patch to be landed. So, please, calm down. :)

Comment 58

[Bill Gianopoulos [:WG9s]]

OK then, I'm not exactly sure how to do this either, but somehow it should have been marked as mfinkle did the r+.  In any event I am doing a build with this backed out now to see if it fixes the issue.  If so I guess there is an issue with this code if feedback is not installed as far as it displaying the build date on nightlies.

Comment 59

[Ryan VanderMeulen [:RyanVM]]

I think the best thing would have been to mention the IRC r+ when posting the patch.

Comment 60

[Bill Gianopoulos [:WG9s]]

Well also I could have waited to launch till I had actually done a build with this backed out to see if it even fixed my issue.

Comment 61

[Théo Chevalier]

(In reply to Ryan VanderMeulen from comment #59)
> I think the best thing would have been to mention the IRC r+ when posting
> the patch.

Yeah, sorry, I was only thinking to the person who will land it, and only wrote r=mfinkle into the patch, but now I know. ;)

WG9s: It's possible, but it seems weird to me since I only add text under this build ID, and I've not modified the CSS for XUL. You only noticed it for XUL, right?

Comment 62

[Bill Gianopoulos [:WG9s]]

This wa confirmed as the cause of the issue via backout.

Comment 63

[Bill Gianopoulos [:WG9s]]

What I observed was that a mozilla-central android build with no add-ons installed did not show a build date where a build from http://www.wg9s.com/mozilla/firefox/ (which has this patch backed out) does.

Comment 64

[Bill Gianopoulos [:WG9s]]

Backing this out did fix my issue.