591368 - [Feedback] Get a Feedback XPI in to mobile-browser

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

General tracking bug for getting the Feedback XPI landed in mobile-browser

Comment 1

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

Turns out, this is not the same XPI as shipped with Firefox

Comment 2

[Aakash Desai [:aakashd]]

Attachment: Feedback Pane in Betas

- Specifications -

Pane: The pane is a only added for users on the beta channel. It should persist no matter if that beta user is in a beta, rc or released version of mobile firefox.

Buttons:
+ Give Feedback - Two buttons that link to the mobile versions of the happy/sad forms on Firefox Input. The links for these pages are: 

smiley face - http://m.input.mozilla.com/en-US/happy
sad face -  http://m.input.mozilla.com/en-US/sad 


+ Respond to an Inquiry - A link to Mozilla's surveygizmo survey submission pages. The button should update to show the number of available surveys (if there's more than one). If there's only one, simply show "Go to Page" as the text. Marketing has been using these APIs within Test Pilot in the Firefox 4 Beta Program, so uses can be found there. A list of APIs available from the site can also be seen here: http://www.surveygizmo.com/survey-support/tutorials/online-survey-api-integration-cms-intranet/#whatcani .

+ Features of Interest! - The button should link directly to the latest MobiFx Beta Field Guide. Madhava should have more to say here on the content and the link for it.

+ About this Beta - The button should link to the release notes page. Underneath the "About this Beta" text should show the Version that the user is on. An example of this should be under the Start Page menu item when a user clicks on "Use Current Page" (ie. the current page's title will be shown under "Start Page").

+ Force Addons Compatibility - A slider that allows users to set the "extensions.checkCompatibility" pref to true/false (false by default). We'll need a restart notification bar pop-up after a change.

+ Enable the Error Console - A slider that allows users to set the "browser.console.showInPanel" pref to true/false (false by default). We'll need a restart notification bar pop-up after a change.

Comment 3

[Aakash Desai [:aakashd]]

Icons used in Firefox 4 for the happy/sad faces: http://svn.mozilla.org/design/projects/firefoxFeedback/design/

Comment 4

[Aakash Desai [:aakashd]]

Attachment: happy face

Comment 5

[Aakash Desai [:aakashd]]

Attachment: sad face

Comment 6

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

Attachment: patch

This patch adds the "Beta" panel, including:
* Homemade button for accessing the panel
* Rows for everything in the spec, except the "Respond to an Inquiry" feature. I'm not sure I want to get into that for the first beta of Fennec (or at all - what's the value?)
* Happy, Sad, About buttons go to the right links.
* Features of Interest (not a great string, btw) goes to Madhava's Field Guide for Fennec 1.1 until we get something real - or kill this row (my vote).

This patch only builds the add-on if a beta channel is being created, just like Firefox. To test it, you either build a beta channel, or change the "beta" in the Makefile.in to "default" _or_ use the XPI I am going to post.

Comment 7

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

Attachment: feedback.zip

Unpack this in the "fennec/extensions" folder to test the add-on.

Comment 8

[Aakash Desai [:aakashd]]

After talking to Caitlin, she's fine without not having "Respond to an inquiry" until there's a list of things to create surveys on for the pane.

Comment 9

[Aakash Desai [:aakashd]]

Things to Note:
- Capitalization issues on: "Give Feedback", "Force Add-ons Compatibility"
- Change the title of the pane from "Beta Feedback" to "Beta Tester Tools"
- Change "Features of Interest" to "Topics of Interest" and link to http://planet.firefox.com/mobile (unless Madhava disagrees here)

Comment 10

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

I was actually going to comment on using Sentence Case, instead of Title Case, since these are labels not menus or buttons - but, as an add-on,  I'm not really too concerned as long as we are consistent.

"Topics of Interest" still seems forced, and a little like a detective paperback novel. We could just call it what it is: "&brandShortName; Blog Posts"

Comment 11

[Aakash Desai [:aakashd]]

In terms of case, we're on the same page. We use the sentence case in the preferences pane, so using the that same method in the beta pane works for me.

"Firefox Blog Posts" lacks direction for why a user should click on it. "Topics of Interest" doesn't really work either. So, let's just can it until a decent field guide is available for users to read through.

Comment 12

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

Attachment: patch

Patch for review. This patch is the same as the last, but tweaks the stings and removes "Features of Interest", as discussed.

This patch build the add-on only for beta channels, so a normal developer build will not create it. If you do want to build the add-on, tweak your mozconfig to build a beta, or change:

ifeq (beta,$(MOZ_UPDATE_CHANNEL))
to
ifeq (default,$(MOZ_UPDATE_CHANNEL))

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I had a hang in closing fennec when turning off the debugging and disabling the extension then restarting fennec.

Comment 14

[Matt Brubeck (:mbrubeck)]

Comment on attachment 474156 [details] [diff] [review]
patch

>+        // Notify all windows that an application quit has been requested
>+        var cancelQuit = Cc["@mozilla.org/supports-PRBool;1"].createInstance(Ci.nsISupportsPRBool);
>+        Services.obs.notifyObservers(cancelQuit, "quit-application-requested", "restart");
>+  
>+        // If nothing aborted, quit the app
>+        if (cancelQuit.data == false) {
>+          let appStartup = Cc["@mozilla.org/toolkit/app-startup;1"].getService(Ci.nsIAppStartup);
>+          appStartup.quit(Ci.nsIAppStartup.eRestart | Ci.nsIAppStartup.eAttemptQuit);
>+        }

We have the same basic code duplicated in at least three other places.  Can we put it in a common location (Browser.restart?) and make this extension use it, along with the other callers?

>+        <em:minVersion>1.1</em:minVersion>
>+        <em:maxVersion>2.*</em:maxVersion>

...of course, if we add new APIs like Browser.restart and use them here, we'd have to change the minVersion.

>+    <em:name>Feedback</em:name>
>+    <em:description>Help make Firefox better by giving feedback.</em:description>

What's our plan for localization here?

Should we rename it something like "Beta Tester Tools" to make it more obviously connected to the "

Comment 15

[Matt Brubeck (:mbrubeck)]

[continued - bugzilla didn't like the greek letter in my comment]

Should we rename it something like "Beta Tester Tools" to make it more obviously connected to the "beta" panel icon?

r=mbrubeck, feel free to address the above before checkin or file followups as appropriate.

Comment 16

[Aakash Desai [:aakashd]]

WFM. I think the description of the add-on is fine as is.

Comment 17

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

(In reply to comment #14)
> Comment on attachment 474156 [details] [diff] [review]
> patch
> 
> >+        // Notify all windows that an application quit has been requested

> >+          appStartup.quit(Ci.nsIAppStartup.eRestart | Ci.nsIAppStartup.eAttemptQuit);
> >+        }
> 
> We have the same basic code duplicated in at least three other places.  Can we
> put it in a common location (Browser.restart?) and make this extension use it,
> along with the other callers?

Filing a new bug for this. We might need to implement FUEL in Fennec and FUEL has a nice Application.restart method.

> 
> >+        <em:minVersion>1.1</em:minVersion>
> >+        <em:maxVersion>2.*</em:maxVersion>
> 
> ...of course, if we add new APIs like Browser.restart and use them here, we'd
> have to change the minVersion.

Whoops, this was a leftover debugging change. I changed minVersion to 2.0b1pre

> >+    <em:name>Feedback</em:name>
> >+    <em:description>Help make Firefox better by giving feedback.</em:description>
> 
> What's our plan for localization here?

We should be able to localize the add-on and the RDF (via https://developer.mozilla.org/en/Localizing_extension_descriptions), but I am not sure if L10N wants to or not.

> Should we rename it something like "Beta Tester Tools" to make it more
> obviously connected to the "

Renamed, but kept the add-on ID as is.

Comment 18

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

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

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100920 Firefox/4.0b7pre Fennec/2.0b1pre

Litmus tests created : 
12917, 12916, 12915, 12914, 12913, 12912, 12911, 12910, 12909

Smoke tests to ensure that each of the buttons functions as designed.
Android blocked by bug 597041