Last Comment Bug 731546 - Add link to marketplace on about:home
: Add link to marketplace on about:home
Status: VERIFIED FIXED
[about-home][qa!]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 14
Assigned To: Frank Yan (:fryn)
:
:
Mentors:
Depends on: 711157 738642 738646
Blocks: 614106
  Show dependency treegraph
 
Reported: 2012-02-29 03:20 PST by :Felipe Gomes (needinfo me!)
Modified: 2013-12-27 14:23 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
verified


Attachments
patch w/ placeholders; waiting for official icons & branding (9.48 KB, patch)
2012-03-01 21:35 PST, Frank Yan (:fryn)
no flags Details | Diff | Splinter Review
screenshot of patch w/ placeholders (260.43 KB, image/png)
2012-03-01 21:38 PST, Frank Yan (:fryn)
mak77: feedback+
Details
patch w/ placeholder icon (9.79 KB, patch)
2012-03-23 07:23 PDT, Frank Yan (:fryn)
mak77: review+
dolske: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description :Felipe Gomes (needinfo me!) 2012-02-29 03:20:27 PST
The new about:home should have a button to open the apps marketplace; this will be the main entry point for Web Apps discovery in the product
Comment 1 Frank Yan (:fryn) 2012-03-01 21:35:35 PST
Created attachment 602258 [details] [diff] [review]
patch w/ placeholders; waiting for official icons & branding
Comment 2 Frank Yan (:fryn) 2012-03-01 21:38:33 PST
Created attachment 602259 [details]
screenshot of patch w/ placeholders
Comment 3 Marco Bonardo [::mak] 2012-03-02 04:56:44 PST
Comment on attachment 602259 [details]
screenshot of patch w/ placeholders

I absolutely have to give a positive feedback on the placeholder :)
Comment 4 Francesco Lodolo [:flod] 2012-03-15 09:24:08 PDT
+<!ENTITY abouthome.appsButton.label      "Martellplace">

MartellPlace??
Comment 5 Francesco Lodolo [:flod] 2012-03-15 09:30:01 PDT
(In reply to Francesco Lodolo [:flod] from comment #4)
> +<!ENTITY abouthome.appsButton.label      "Martellplace">
> 
> MartellPlace??

Never mind, I was checking the strings and didn't see the screenshot ;-)
Comment 6 Frank Yan (:fryn) 2012-03-23 07:23:03 PDT
Created attachment 608697 [details] [diff] [review]
patch w/ placeholder icon

Dolske asked me to write this patch and hopefully have it landed today or tomorrow.

The new ordering of the icons is ui-approved by limi and shorlander.

The requirement from product is to be able to turn on the marketplace icon using a pref. The patch checks for the hidden pref "browser.aboutHome.apps".
This pref is not for customization purposes. It simply exists by the apps team's request in the likely event that the marketplace launch happens between Firefox version launches. The pref check and hidden attribute will be removed once the marketplace launches.

The URL has been finalized as https://marketplace.mozilla.org/.

The marketplace icon is still a placeholder, as we have not been provided a final icon or silhouette from which to produce a matching icon for about:home. I will file a followup shortly for the final icon.
Comment 7 Marco Bonardo [::mak] 2012-03-23 07:43:08 PDT
Comment on attachment 608697 [details] [diff] [review]
patch w/ placeholder icon

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

(In reply to Frank Yan (:fryn) from comment #6)
> The pref check and hidden attribute will
> be removed once the marketplace launches.

The patch looks good.

My doubt regarding this, is about branding.  Is the Marketplace something to be considered Firefox only? I wonder if for foreign branding we may have to always keep the button hidden option.
If so I'd probably consider inverting the option, so usually show the button unless a disable option is true and have other brands set the pref, than us.

::: browser/base/content/browser.js
@@ +2707,5 @@
>   */
>  function BrowserOnAboutPageLoad(document) {
>    if (/^about:home$/i.test(document.documentURI)) {
> +    // XXX when Marketplace is launched, remove this statement and the hidden
> +    // attribute set on the apps button in aboutHome.xhtml

I'd prefer if we'd have a bug tracking this, and the bug number be reported here instead of the n-th XXX comment that nobody will ever fix.
Comment 8 Dão Gottwald [:dao] 2012-03-23 07:50:09 PDT
(In reply to Marco Bonardo [:mak] from comment #7)
> My doubt regarding this, is about branding.  Is the Marketplace something to
> be considered Firefox only? I wonder if for foreign branding we may have to
> always keep the button hidden option.

What exactly is your concern here? I don't see a meaningful dependency between official branding and the web apps marketplace.
Comment 9 Frank Yan (:fryn) 2012-03-23 07:56:05 PDT
(In reply to Marco Bonardo [:mak] from comment #7)
> > +    // XXX when Marketplace is launched, remove this statement and the hidden
> > +    // attribute set on the apps button in aboutHome.xhtml
> 
> I'd prefer if we'd have a bug tracking this, and the bug number be reported
> here instead of the n-th XXX comment that nobody will ever fix.

I just filed bug 738646 and will add that to the comment when landing this.
I won't forget about it as I strongly dislike unnecessary prefs.

(In reply to Dão Gottwald [:dao] from comment #8)
> (In reply to Marco Bonardo [:mak] from comment #7)
> > My doubt regarding this, is about branding.  Is the Marketplace something to
> > be considered Firefox only? I wonder if for foreign branding we may have to
> > always keep the button hidden option.
> 
> What exactly is your concern here? I don't see a meaningful dependency
> between official branding and the web apps marketplace.

Marco and I just discussed his question on IRC. I don't think Marco has any major concern about this anymore. Feel free to read the #fx-team scrollback for more details.
Comment 10 Marco Bonardo [::mak] 2012-03-23 08:02:40 PDT
Comment on attachment 608697 [details] [diff] [review]
patch w/ placeholder icon

ok, let's say we don't care till someone brings more meaningful reasoning about that.
Comment 11 Frank Yan (:fryn) 2012-03-23 08:31:46 PDT
Pushed to fx-team with bug number for removing temporary pref in comment.
https://hg.mozilla.org/integration/fx-team/rev/5ff4f574298e

Thank you for the quick review, Marco! :)
Comment 12 Frank Yan (:fryn) 2012-03-23 08:33:09 PDT
Comment on attachment 608697 [details] [diff] [review]
patch w/ placeholder icon

[Approval Request Comment]
User impact if declined: no marketplace link.
Risk to taking this patch: none.
String changes made by this patch: none.
Comment 14 Jason Smith [:jsmith] 2012-04-13 09:17:05 PDT
To clarify - To test this, I need to enable browser.aboutHome.apps to true and check the about:home has a marketplace button with the appropriate icon. Then clicking that button should take me to marketplace.mozilla.org. Is this correct?
Comment 15 Jason Smith [:jsmith] 2012-04-13 09:23:07 PDT
Actually, figured it out by enabling that pref. Looks like about:home does show the marketplace link in the bottom toolbar. The button does go to marketplace.mozilla.org. However, there's two outstanding issues for this bug:

- The icon shown is a giant X - Not the marketplace icon
- The code for this is enabled on FF 13, but should instead be enabled on FF 14 or later only
Comment 16 Asa Dotzler [:asa] 2012-04-14 15:17:43 PDT
I don't think there's a problem with this code being enabled in Firefox 13 (I think that's the goal, actually) if the pref is off by default as expected. We will use the add-on hotfix to enable the button and add the appropriate image unless we can get that image sooner.
Comment 17 Jason Smith [:jsmith] 2012-05-11 08:18:04 PDT
Verified on Nightly.
Comment 18 Paul Silaghi, QA [:pauly] 2012-05-11 08:19:24 PDT
Marketplace button is displayed in about:home when browser.aboutHome.apps pref is set on true.
Verified fixed on FF 13b3:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:13.0) Gecko/20100101 Firefox/13.0

Note You need to log in before you can comment on or make changes to this bug.