568896 - Update about:firstrun for Fennec 2.0a1

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Caitlin Looney]

Hey Mark,

Can you change Weave Sync to Firefox Sync on the local first run page? All remote instances are being taken care of.

Thanks!

Comment 1

[Axel Hecht [:Pike]]

Not really if we want to ship soon. Weave/Sync shows up in a few places in l10n, and thus we'd break string freeze hard, and basically start shipping RC 1 from scratch.

Comment 2

[Caitlin Looney]

Just spoke with Mark and Stuart. If this is going to break things, let's get it changed for 1.1.1

Comment 3

[Caitlin Looney]

What's the status of this, Mark? I'd really like to get this swapped out ASAP.

Comment 4

[Caitlin Looney]

I haven't heard from anyone here. The first run in Alpha is inaccurate now that Sync is built-in and the starter kit is 50% compatible. 

Can we do something simple letting people know where to login with Firefox Sync credentials as first run and a bit about the new Start page? If I can give you a bit of copy and we keep the Take a Tour, is this something we can get in by release?

Comment 5

[[:fabrice] Fabrice Desré]

Caitlin,

Do we have mockups of the new start page? I guess you'd like the Firefox sync section to open the pref panel and show the sync preferences?

Comment 6

[Caitlin Looney]

Thanks.

Wes has my new copy and we're using the images from the Firefox.com/m download page. We're all changing the header and replacing Features link with Rel Notes. I think this should actually look and behave really well. Thanks everyone!

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Cleanup for Alpha

This removes the intro gif which, doesn't play anymore. Also replaces the images in the three options.

The options are changed so that Weave->Sync, and points to the prefs pane. The second Addons button points to the Addons prefs. And the third points you to about:home. Also removed a few other links and added a link to the release notes (page doesn't exist yet?)

Comment 8

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

Comment on attachment 467475 [details] [diff] [review]
Cleanup for Alpha

>+          let syncArea = win.document.getElementById("prefs-sync");
>+          if(prefsBox && syncArea) {
>+            prefsBox.ensureElementIsVisible(syncArea);
>+          }

No {} needed for one-liners

>-<!ENTITY firstrun.features             "See More Features">
>+<!ENTITY firstrun.heading              "Welcome to &brandShortName;">
>+<!ENTITY firstrun.sync.title          "Get Firefox Sync">
>+<!ENTITY firstrun.sync.description    "Share your &brandShortName; history, passwords, bookmarks &amp; tabs between your desktop &amp; mobile">
>+<!ENTITY firstrun.starter.title        "Get Addons">

Align text please

Comment 9

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

pushed:
http://hg.mozilla.org/mobile-browser/rev/d2e084002d97

Comment 10

[Francesco Lodolo [:flod]]

Sorry to always bitch about this kind of things but changing strings without changing the key name is plainly wrong.

Comment 11

[Axel Hecht [:Pike]]

Reopening. And yeah, nominating comment 8 as review comment of the week.

Comment 12

[Aakash Desai [:aakashd]]

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=12260 updated

Comment 13

[Wesley Johnston (:wesj)]

Attachment: String fixes

Fixes up string and id names. Uses the same logic as the about.xhtml page for finding the release notes url as well.

Comment 14

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

Comment on attachment 467797 [details] [diff] [review]
String fixes

>diff -r d2e084002d97 chrome/content/firstrun/firstrun.xhtml
>--- a/chrome/content/firstrun/firstrun.xhtml	Thu Aug 19 15:21:07 2010 -0400
>+++ b/chrome/content/firstrun/firstrun.xhtml	Fri Aug 20 09:05:58 2010 -0700
>@@ -69,22 +69,21 @@
>       <h1><img src="chrome://branding/content/logoWordmark.png"/></h1>
>       <p>&firstrun.heading;</p>
>     </div>
>   
>     <div id="content">
>       <h2></h2>
>       <ul id="recommended">
>         <li id="sync"><a href="javascript:loadSync();"><b>&firstrun.sync.title;</b> <span>&firstrun.sync.description;</span></a></li>
>-        <li id="starter"><a href="javascript:loadAddons();"><b>&firstrun.starter.title;</b> <span>&firstrun.starter.description;</span></a></li>
>-        <li id="tour"><a href="about:home"><b>&firstrun.tour.title;</b> <span>&firstrun.tour.description;</span></a></li>
>+        <li id="addons"><a href="javascript:loadAddons();"><b>&firstrun.addons.title;</b> <span>&firstrun.addons.description;</span></a></li>
>+        <li id="home"><a href="about:home"><b>&firstrun.home.title;</b> <span>&firstrun.home.description;</span></a></li>
>       </ul>
> 
>-      <p id="features"><a href="https://www.mozilla.com/en-US/mobile/2.0a1/releasenotes/">&firstrun.features;</a></p>
>-
>+      <p id="relNotes"><a id="releaseNotesURL">&firstrun.relNotes.label;</a></p>
>       <script type="application/javascript;version=1.8"><![CDATA[
>         let Ci = Components.interfaces, Cc = Components.classes, Cu = Components.utils;
>         let gChromeWin = null;
>         
>         function getChromeWin() {
>           if (!gChromeWin) {
>             gChromeWin = window
>                     .QueryInterface(Ci.nsIInterfaceRequestor)
>@@ -107,16 +106,18 @@
>           if (prefsBox && syncArea)
>             prefsBox.ensureElementIsVisible(syncArea);
>         }
> 
>         function loadAddons() {
>           let win = getChromeWin();
>           win.BrowserUI.showPanel("addons-container");
>         }
>+        
>+        

Remove the unneeded whitespace

>+  <script type="application/javascript;version=1.8"><![CDATA[
>+    // get URLs from prefs
>+    try {
>+      let formatter = Components.classes["@mozilla.org/toolkit/URLFormatterService;1"]
>+                                .getService(Components.interfaces.nsIURLFormatter);

>+  ]]></script>

Why not move the script to live in the same block? Then you can use Cc and Ci too

>diff -r d2e084002d97 locales/en-US/chrome/firstrun.dtd

I am looking at the original source when making these comments, not just the current source.

> <!ENTITY firstrun.title                "Welcome to &brandShortName;">
> <!ENTITY firstrun.heading              "Welcome to &brandShortName;">

"firstrun.heading" used to be ""To help get you started, ..." so you need to change the entity name for it too. You could just drop it and use "firstrun.title" twice in the HTML. If not, use "firstrun.heading2"

>+<!ENTITY firstrun.addons.title         "Get Addons">

"Get Add-ons"


I hope this review wins some kind of award too!

Comment 15

[Wesley Johnston (:wesj)]

Attachment: More strings

Comment 16

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

pushed:
http://hg.mozilla.org/mobile-browser/rev/88c8ffbb40dd

Comment 17

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b4pre) Gecko/20100823 Namoroka/4.0b4pre Fennec/2.0a1pre

and

Mozilla/5.0 (Android; U; Linux armv71; en-US; rv:2.0b4pre) Gecko/20100823 Namoroka/4.0b4pre Fennec/2.0a1pre