Make the fragment a map of {guid: add-on name} in the Discovery Pane

VERIFIED FIXED in mozilla1.9.3a5

Status

()

defect
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: jbalogh, Assigned: mossop)

Tracking

Trunk
mozilla1.9.3a5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rewrite], )

Attachments

(1 attachment)

Reporter

Description

9 years ago
Or the other way around.  The direction doesn't matter.

I need the add-on names for the Support section of about:addons[1].  It displays all the user's add-ons with a link to a support page on AMO.  We can't ask AMO for a guid => name map because that hurts privacy.

[1]: http://docs.google.com/Doc?docid=0Acwo2Bn17-PrZGZudHRobnJfNDJ0cTV0NjRnZA&hl=en&pli=1 (at the bottom)
Took me awhile to figure out this is for the Discover pane.

Moving it over to the meta bug for that.
Blocks: 558158
No longer blocks: 550048
Whiteboard: [rewrite]
Summary: [about:addons] Make the fragment a map of {guid: add-on name} → Make the fragment a map of {guid: add-on name} in the Discovery Pane
A lot of the Discovery Pane is ready now, but not having this hash makes a lot of the testing we were hoping to do with the scaffolding moot.

When do you think this will land?

Also, it's probably a good idea to include more than just the GUID and name. Let's do name, version, and status for each GUID.
So do you want this as a map of id: {name, version, status} or an array of {id, name, version, status}?

Also what is status? The status we send with update requests or something else?
Up to Jeff but I'd think the former. And status is the same as update requests, such as userEnabled, blocklisted, etc.
Reporter

Comment 5

9 years ago
(In reply to comment #3)
> So do you want this as a map of id: {name, version, status} or an array of {id,
> name, version, status}?

{<guid>: {name: <name>, version: <version>, ...}} please.  Thanks!

Are there any other details that you think might be interesting to us?  I'd like to get everything we can up front before we're stuck with what's in Firefox 4.
(In reply to comment #4)
> Up to Jeff but I'd think the former. And status is the same as update requests,
> such as userEnabled, blocklisted, etc.

Rather than replicating the status string and then presumably having JS in the page to parse it into something useful I think it would be easier both on our and your code for us to include specific properties in the object, userDisabled, isCompatible, isBlocklisted.

(In reply to comment #2)
> When do you think this will land?

Going to try to make sure it is done by the end of the week.
The patch I have working for this is generating URLs like this:

https://services.addons.mozilla.org/en-US/firefox/discovery/3.7a5pre/Darwin#{%22inspector@mozilla.org%22:{%22name%22:%22DOM%20Inspector%22,%22version%22:%222.0.0%22,%22type%22:%22extension%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22timerfire@oxymoronical.com%22:{%22name%22:%22Timer%20Fire%22,%22version%22:%221.5%22,%22type%22:%22extension%22,%22userDisabled%22:false,%22isCompatible%22:false,%22isBlocklisted%22:false},%22{75739dec-72db-4020-aa9a-6afa6744759b}%22:{%22name%22:%22Extension%20Developer%22,%22version%22:%220.3.0.20060726%22,%22type%22:%22extension%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{8620c15f-30dc-4dba-a131-7c5d20cf4a29}%22:{%22name%22:%22Nightly%20Tester%20Tools%22,%22version%22:%222.0.3%22,%22type%22:%22extension%22,%22userDisabled%22:false,%22isCompatible%22:false,%22isBlocklisted%22:false},%22{F8A55C97-3DB6-4961-A81D-0DE0080E53CB}%22:{%22name%22:%22Download%20Manager%20Tweak%22,%22version%22:%220.9.2%22,%22type%22:%22extension%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{972ce4c6-7e08-4474-a285-3208198ce6fd}%22:{%22name%22:%22Default%22,%22version%22:%223.7a5pre%22,%22type%22:%22theme%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{94f5cfb8-19e0-bf48-8f9f-ac53e023daef}%22:{%22name%22:%22Test%20Plug-in%22,%22version%22:%221.0.0.0%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{c6148506-96cc-ba40-800c-55ce6c07ef49}%22:{%22name%22:%22Default%20Plugin%22,%22version%22:%222.0%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{a8d57c58-0ace-2e44-89db-449f30980e93}%22:{%22name%22:%22Facebook%20Plug-In%22,%22version%22:%221.0%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{f14e330a-7b2b-a146-b395-1a9cd989a734}%22:{%22name%22:%22iPhotoPhotocast%22,%22version%22:%227.0%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{20bf7604-a61e-4c4d-ba95-503ff62e15ed}%22:{%22name%22:%22Silverlight%20Plug-In%22,%22version%22:%223.0.50106.0%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{7b1ee460-2e83-bb47-a666-6d899ffca5c4}%22:{%22name%22:%22Flip4Mac%20Windows%20Media%20Plugin%202.3.2%20%22,%22version%22:%222.3.2.6%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{91bf27f7-c749-914f-a088-8dcbe83c2e5c}%22:{%22name%22:%22Shockwave%20Flash%22,%22version%22:%2210.1.53.38%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false},%22{df72c059-2261-c044-9fc1-d00d9a633c08}%22:{%22name%22:%22QuickTime%20Plug-in%207.6.6%22,%22version%22:%227.6.6%22,%22type%22:%22plugin%22,%22userDisabled%22:false,%22isCompatible%22:true,%22isBlocklisted%22:false}}

You'll note that there is a little encoding going on there, not sure there is an easy way around that on my end and it's probably easy to solve on your side. It also contains all types of add-ons, plugins included so if you don't care about those you'll need to filter out only the ones you do care about.

Does this look ok?
URLs like that are going to make our front end caches pretty much worthless.  Do we really need all this info and can we load it via AJAX instead?
(In reply to comment #8)
> URLs like that are going to make our front end caches pretty much worthless. 
> Do we really need all this info and can we load it via AJAX instead?

Jeff points out this is all behind a hash so I take back everything bad that I said. :)
Reporter

Comment 10

9 years ago
(In reply to comment #7)
> The patch I have working for this is generating URLs like this:
> Does this look ok?

That's beautiful. JSON.parse(location.hash.slice(1)) handles it fine.
Posted patch patch rev 1Splinter Review
Fairly straightforward stuff, just appends the data to the url we load.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #445225 - Flags: review?(bmcbride)
Comment on attachment 445225 [details] [diff] [review]
patch rev 1

That is an epic hash :)

Do we want to eventually force a refresh/change of hash when a new addon is installed (from there or another tab) while the Discover pane is the active pane?
Attachment #445225 - Flags: review?(bmcbride) → review+
Reporter

Comment 13

9 years ago
(In reply to comment #12)
> (From update of attachment 445225 [details] [diff] [review])
> That is an epic hash :)

Is there any limit on the length of the hash/url that we could run into?

> Do we want to eventually force a refresh/change of hash when a new addon is
> installed (from there or another tab) while the Discover pane is the active
> pane?

I wouldn't mind (that would trigger onhashchange, right?).  If the discovery pane knows the add-ons have changed, it can offer to update the personal recommendations.  Otherwise we'll be looking for changes next time the page is reloaded.  How often would that happen?
(In reply to comment #13)
> (In reply to comment #12)
> > (From update of attachment 445225 [details] [diff] [review] [details])
> > That is an epic hash :)
> 
> Is there any limit on the length of the hash/url that we could run into?

I am told that the total url string (url + hash) must be less than 4294967295 characters. The URL here shows us taking something like 250 characters per add-on which means we might only be able to list 17179869 add-ons in the hash, do you think that is enough?

> > Do we want to eventually force a refresh/change of hash when a new addon is
> > installed (from there or another tab) while the Discover pane is the active
> > pane?
> 
> I wouldn't mind (that would trigger onhashchange, right?).  If the discovery
> pane knows the add-ons have changed, it can offer to update the personal
> recommendations.  Otherwise we'll be looking for changes next time the page is
> reloaded.  How often would that happen?

Right now we only regenerate the URL when the add-ons manager is reloaded. We can file a separate bug on updating it while the window is open. I think it's a nice to have but not super important at this stage.
Landed: http://hg.mozilla.org/mozilla-central/rev/1758bbc9aaeb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Assignee

Updated

9 years ago
Depends on: 566626
Added tests for this in bug 603804
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus?
Flags: in-litmus-
(In reply to comment #17)
> Added tests for this in bug 603804

The question here is what has be tested on the discovery pane, especially with the data we get from AOM. Probably we should move the litmus request over there?
(In reply to comment #18)
> (In reply to comment #17)
> > Added tests for this in bug 603804
> 
> The question here is what has be tested on the discovery pane, especially with
> the data we get from AOM. Probably we should move the litmus request over
> there?

All we can test automatically is that the pane attempts to load the correct URL, which is covered. Anything else would require network access which automated tests aren't allowed to do so that the contents of the pane renders correctly etc. needs to be checked. I don't know if AMO has automated tests verifying that already though.
You need to log in before you can comment on or make changes to this bug.