Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Mozilla's released a new session API for BrowserID, which is used by their BrowserID browser add-on for Firefox.
The Drupal BrowserID module should support these new extensions.
Comment | File | Size | Author |
---|---|---|---|
#4 | Support-BrowserID-session-API-1249746-4.patch | 3.79 KB | Samat Jain |
#1 | 0001-Support-BrowserID-session-API-bug-1249746.patch | 3.78 KB | Samat Jain |
Comments
Comment #1
Samat Jain CreditAttribution: Samat Jain commentedPatch posted.
My only qualms: seems like a bad idea to expose user's e-mail address via Javascript (required by the Firefox add-on). AFAIK the user's (Drupal) display name can also appear, but I've left it as the e-mail address for consistency with current use of BrowserID.
Comment #2
Samat Jain CreditAttribution: Samat Jain commentedForgot to mention, I've this code running on my blog [blog.samat.org]. Please feel free to test there!
Comment #3
IceCreamYou CreditAttribution: IceCreamYou commentedSending you to the conventional login page.
is superfluous and should be removed.window.location.href
to redirect/refresh the page, and you useddocument.location.href
. I don't have a strong preference, but it should be consistent.Thanks for the patch!
Comment #4
Samat Jain CreditAttribution: Samat Jain commentedAFAIK is still does support both APIs.
There are no user-facing changes for the old API; the "Sign In" button is still on the user login block/page.
On the backend, the main change is that browserid.js (where most of the changes reside) is now sent with every page request.
Perhaps, but the error message (which is a little cryptic) doesn't tell the user how to finish what they were doing. I've removed the message in the new patch (I agree my wording isn't very good), but still think there should be a better-worded explanation of what happened and how they should fix it.
Fixed in new patch.
I also made a few other minor coding style changes.
Comment #5
IceCreamYou CreditAttribution: IceCreamYou commentedWith your patch applied, browserid.js is only sent for authenticated users. It should also be sent for anonymous users at least when a BrowserID login button is on the page.
Additionally, Drupal's code style standards call for always using curly braces for control structures like if/else even when they're not needed.
How about
Click "Okay" to continue to the normal log-in page.
?Other than that, looks good, and I'll try it and commit it as soon as I can.
Comment #6
Samat Jain CreditAttribution: Samat Jain commentedI think you might be misreading things—browser.js (moved to browserid.info) will be included in every page, while a new Javascript variable Drupal.settings.browserid.email, set in browserid_init() (which is what I think you may be misreading), is only set for authenticated users (i.e. the browser displays they have logged in).
Sorry, I missed that—did you want me to submit a new patch? I think it's only in one place…
AFAIK the error dialog will only have an "OK", so telling them to click it (which they have to, since it's modal and they need to dismiss it) is definitely extraneous. Maybe something like
An error occurred attempting to use your BrowserID, please login conventionally.
so users both know an error occurred and why they are redirected to the conventional login page.Comment #7
IceCreamYou CreditAttribution: IceCreamYou commentedOh, I didn't realize scripts specified in the .info file are always included.
It's in two places:
and
I don't like the word "conventionally" because many users might not know what the convention is. How about
After clicking "OK," you will be redirected to a page where you can log in without BrowserID or try logging in with BrowserID again.
?Anyway, committed the patch with the above changes. Thanks!