780223 - Switch to use navigator.id.watch()

Status: RESOLVED FIXED

(Marketplace Graveyard :: General, defect, P1)

Description

[Wil Clouser [:clouserw]]

BrowserID is switching to navigator.id.watch() as their default recommended implementation.  We want to use it as it adds functionality we need to work with bluevia (namely, them telling us who we are logged in as).

Documentation is at:  https://developer.mozilla.org/en-US/docs/DOM/navigator.id.watch

The new call should completely replace the old call.  :thunder or dcallahan can help with implementation questions.

Comment 1

[Wil Clouser [:clouserw]]

This was fixed in https://github.com/mozilla/zamboni/commit/002ea7aff96602f01b74370db60306a973571b5f

Thanks for the patch!

Comment 2

[JR Conlin [:jrconlin,:jconlin]]

Reopening this bug. I'm seeing unusual behaviors with my test install that may be indicative of a bigger problem.

Notably:
* browserid_support.js not being loaded on some pages.
* Browserid identity panel not always being displayed
* After login, page seems to stall loading identity.

Comment 3

[Wil Clouser [:clouserw]]

These fixes are being tracked in bug 789288 (and pull request already exists there).  Closing since we don't need multiple bugs open for the same thing.

Comment 4

[Kumar McMillan [:kumar]]

I see that this was added to AMO (js/zamboni) but not Marketplace (js/mkt). We need it in the Marketplace: https://github.com/mozilla/zamboni/blob/master/media/js/mkt/browserid.js#L11 Sorry JR for confusing you by having two forks of the code :( That's a long sordid story.

Comment 5

[Wil Clouser [:clouserw]]

JR is working with Kumar on this.

Comment 6

[Kumar McMillan [:kumar]]

What's the status on this? I see that watch landed but maybe it needs some refinement?

Comment 7

[JR Conlin [:jrconlin,:jconlin]]

I've checked the current master (as of 1400PDT 11/01) against my local builds.
Login proceeds correctly, with the exception that developer login routes to "/". This appears to be the expected behavior since the "to" is set to "/" outside of my code being called.
I've not tested the special admin accounts. 

The additional patches add a browserid middleware element to AMO user. Since it appears that MKT is not using that section of code, I've not pushed the additional code.

Comment 8

[Wil Clouser [:clouserw]]

The developer strangeness is a different bug.  Minor oddities aside is the main part of this bug (using .watch() on marketplace.m.o) done?  If so, we can close it.

Comment 9

[JR Conlin [:jrconlin,:jconlin]]

My apologies if i've not followed procedure. I'm used to QA verifying that the bug no longer exists and then closing. Since that doesn't appear to be the case here, I'll close it and presume QA will reopen if there's still an issue.

Comment 10

[Wil Clouser [:clouserw]]

Thanks JR.  We generally mark FIXED when we commit the change (with a link to the commit) and QA moves it from RESOLVED -> VERIFIED.

Comment 11

[Kumar McMillan [:kumar]]

JR, could you also link to the commit that fixed it? We put revision links in the bug to make future spelunking easy. If there are more than one you can add the merge commit or whatever is easiest.

Comment 12

[JR Conlin [:jrconlin,:jconlin]]

fix incorporated as part of fix for 789288:
https://github.com/mozilla/zamboni/pull/457