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

RESOLVED FIXED in Firefox 14

Status

()

defect
P4
normal
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: lmandel, Assigned: tchevalier)

Tracking

unspecified
Firefox 14
ARM
Android
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Telemetry])

Attachments

(4 attachments, 12 obsolete attachments)

139.87 KB, image/jpeg
Details
515.33 KB, image/png
Details
58.58 KB, image/jpeg
ibarlow
: review+
Details
7.91 KB, patch
tchevalier
: review+
Details | Diff | Splinter Review
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.
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?
(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?
(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!
Priority: -- → P4
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?
Posted patch Patch V1 (obsolete) — Splinter Review
A first patch, need feedback.
Attachment #583594 - Flags: feedback?(lmandel)
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?
Attachment #583594 - Flags: feedback?(lmandel) → feedback+
(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?
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 :(
(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/
Madhava - Is adding text to the about:fennec page something that UX wants to review?
Posted image XUL UI (Screenshot)
I finally compile my own Fennec build, here are the screenshots.
Attachment #585520 - Flags: feedback?(lmandel)
Posted image Tablet version V1 (obsolete) —
Attachment #585521 - Flags: feedback?(lmandel)
(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)
(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.
Theo - Thanks for you continued work on this bug. Have you had success building the native UI? Do you need any help moving forward?
(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...
Theo - I understand that Mark reached out to you to offer some assistance. Have you been able to get past your hurdle?
Attachment #585520 - Flags: feedback?(lmandel)
Attachment #585521 - Flags: feedback?(lmandel)
(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. :(
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.
Here you go Theo.
@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?
(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.
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.
Posted image 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. 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?
(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 :)
Posted patch Patch V2 (obsolete) — Splinter Review
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!
Attachment #583594 - Attachment is obsolete: true
Attachment #585521 - Attachment is obsolete: true
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 :)
This first build is crashing at startup on an Android 2.3.4 device.
(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.
(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.
Depends on: 723129
Posted image Screenshot Patch V2 (obsolete) —
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.
Posted patch PatchV3 (obsolete) — Splinter Review
Attachment #598383 - Flags: review?(gavin.sharp)
Attachment #598383 - Flags: review?(gavin.sharp)
(Wrong bug, sorry...)
Attachment #598383 - Attachment is obsolete: true
Assignee: nobody → theo.chevalier11
Posted patch Patch V3 (obsolete) — Splinter Review
I will post a screenshot of this tomorrow.
Attachment #592825 - Attachment is obsolete: true
Attachment #598334 - Attachment is obsolete: true
Posted image Screenshot native UI (obsolete) —
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.
Depends on: 728340
Whiteboard: [Telemetry]
Posted patch Patch V4 (obsolete) — Splinter Review
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.
Attachment #598492 - Attachment is obsolete: true
Attachment #598563 - Attachment is obsolete: true
The black border is not in the patch, it's just to see the position of the div relative to the update button.
Attachment #598662 - Flags: review?(ibarlow)
Posted patch Patch V5 (obsolete) — Splinter Review
Improvement: Underline the Mozilla link, because we can't see this is a link.
Attachment #598629 - Attachment is obsolete: true
Comment on attachment 598662 [details]
Native UI (screenshot)

This looks great, Théo, thanks!
Attachment #598662 - Flags: review?(ibarlow) → review+
Attachment #599130 - Flags: review?(mark.finkle)
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."
Depends on: 728350
Posted patch Patch V6 (obsolete) — Splinter Review
Attachment #599130 - Attachment is obsolete: true
Attachment #599130 - Flags: review?(mark.finkle)
Attachment #599635 - Flags: review?(mark.finkle)
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>"
Attachment #599635 - Flags: review?(mark.finkle) → review+
Posted patch Patch V6 (indent update) (obsolete) — Splinter Review
Oops, corrected!
Attachment #599635 - Attachment is obsolete: true
Whiteboard: [Telemetry] → [Telemetry][checkin-needed]
Attachment #590799 - Attachment is obsolete: true
Status: NEW → ASSIGNED
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.
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. :)
Posted patch Patch V7Splinter Review
Ok now it's good.
Attachment #606322 - Attachment is obsolete: true
Attachment #598662 - Attachment description: Screenshot patch V4 → Native UI (screenshot)
Attachment #585520 - Attachment description: Phone version V1 → XUL UI (Screenshot)
Keywords: checkin-needed
Whiteboard: [Telemetry][checkin-needed] → [Telemetry]
https://hg.mozilla.org/integration/mozilla-inbound/rev/77123b152b5c

I assume you'll be requesting approval-aurora too.
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → Firefox 14
(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.
Blocks: 737596
No longer blocks: 699806
https://hg.mozilla.org/mozilla-central/rev/77123b152b5c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Is there a specific reason for that starting blank space in aboutPage.telemetryStart?
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.
(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.
Depends on: 745567
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???
Patch author identified as  Théo Chevalier [:tchevalier] reviewer identified as theo.chevalier11: review+ what kind of review process is that?
(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. :)
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.
I think the best thing would have been to mention the IRC r+ when posting the patch.
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.
(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?
This wa confirmed as the cause of the issue via backout.
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.
Backing this out did fix my issue.
Depends on: 746527
You need to log in before you can comment on or make changes to this bug.