Problem/Motivation
OpenID login does not degrade with js turned off
Proposed resolution
I suggest turning the two links into real links (i.e. without return false in the click handler), linking to #openid and #, respectively. The openid.js file should check for location.hash on load and display the OpenID login field, if the user accessed the #openid URL directly. This allows users to bookmark the page with the visible OpenID login field.
Remaining tasks
patch needs to be tested
User interface changes
set open-id links as real external links
API changes
'data' => l(t('Log in using OpenID'), '#')
changed to
'data' => l(t('Log in using OpenID'), '#openid-login', array('external' => TRUE))
and
'data' => l(t('Cancel OpenID login'), '#')
to
'data' => l(t('Cancel OpenID login'), '#', array('external' => TRUE))
javascript now looks for openid-login hash
if ($('#edit-openid-identifier').val() || location.hash == '#openid-login')
Original post
As per title, the "login with openid" link is http://example.com/%23 --> the %23 is a weirdly encoded # sign.
And, looking at this, this means that people with JavaScript turned off can't login with openid. Suggestion is that in the absence of JavaScript, it gets turned into a link that goes to http://example.com/openid/login which will contain an openid-only login.
Comment | File | Size | Author |
---|---|---|---|
#19 | openid-link-2-6.x.patch | 1.43 KB | danillonunes |
#16 | openid-link-2.patch | 1.39 KB | c960657 |
#11 | openid-link-1.patch | 2.67 KB | c960657 |
#5 | openid.patch | 632 bytes | danillonunes |
#3 | openid_link.patch | 1.07 KB | Pancho |
Comments
Comment #1
walkah CreditAttribution: walkah commentedDid you try with JS off? it absolutely does degrade... the link doesn't show with JS off, and the form field is displayed.
Comment #2
John Morahan CreditAttribution: John Morahan commentedIt does degrade. But that %23 can't be by design, can it?
Comment #3
PanchoNo, that's not by design, but there seems to be no work-around as we want to keep using the l()-function. The alternative would be without a hash, then it's at least not distorted. See my patch.
Comment #4
John Morahan CreditAttribution: John Morahan commentedDo we even need l() here? From http://api.drupal.org/api/function/l/6 -
We don't need these features here if we simply want
<a href="#">
. Could we just output that directly?Comment #5
danillonunes CreditAttribution: danillonunes commentedThe trouble with this bug is no detected by who doesn't have javascript, but by who has js and are accessing a slow site. As the behaviors are not appended until the DOM is ready, if you click the link when the page is loading you is redirected to an 404 page.
Patch #3 don't fix totally since if you click the link before the page finish loading, your browser refresh the page... so... you need to wait it load again to login.
I make a patch using a solution proposed in api.drupal.org to create hash links with l() function.
Comment #6
c960657 CreditAttribution: c960657 commentedI suggest turning the two links into real links (i.e. without
return false
in the click handler), linking to #openid and #, respectively. The openid.js file should check for location.hash on load and display the OpenID login field, if the user accessed the #openid URL directly. This allows users to bookmark the page with the visible OpenID login field.Comment #7
Irous CreditAttribution: Irous commentedI've just started getting the http://example.com/%23 problem when trying to log in via openID on my site. I've realised that this started happening when I enabled the "Optimize JavaScript files" in the Performance (caching) options in admin. Once I turned javascript caching off again, openID was working fine.
Comment #8
srobert72 CreditAttribution: srobert72 commentedSubscribing
Comment #9
srobert72 CreditAttribution: srobert72 commentedPatch #5 works well for me
Comment #10
srobert72 CreditAttribution: srobert72 commentedDrupal 6.17 released today.
I repatched manually openid.module with patch #5.
All works with Drupal 6.17
Comment #11
c960657 CreditAttribution: c960657 commentedThis implements (almost) what I suggested in #6. This will make the link works as a link if you right-click and chose "Open in new window...".
Or is this over-thinking?
Comment #13
c960657 CreditAttribution: c960657 commented#11: openid-link-1.patch queued for re-testing.
Comment #14
Daniel Norton CreditAttribution: Daniel Norton commentedBumped priority. Creating invalid links is definitely not a “cosmetic” issue.
Comment #15
wojtha CreditAttribution: wojtha commentedSubscribing
Comment #16
c960657 CreditAttribution: c960657 commentedComment #17
wojtha CreditAttribution: wojtha commentedTested
Comment #18
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x.
Sounds like this needs to get backported to Drupal 6 so I'm marking this as 'needs work' against 6.x-dev.
Comment #19
danillonunes CreditAttribution: danillonunes commentedHere is the patch for 6.x.
To test it, I just wrap the
Drupal.attachBehaviors(this);
in misc/drupal.js:276 with asetTimeout(function() { Drupal.attachBehaviors(this); }, 5000);
, in order to simulate a slow execution of behaviors.Comment #19.0
jasonrichardsmith@gmail.com CreditAttribution: jasonrichardsmith@gmail.com commentedsummary added by webchicks request
Comment #19.1
jasonrichardsmith@gmail.com CreditAttribution: jasonrichardsmith@gmail.com commentedadded blockquote tag
Comment #19.2
jasonrichardsmith@gmail.com CreditAttribution: jasonrichardsmith@gmail.com commentedfixed block quote again
Comment #19.3
jasonrichardsmith@gmail.com CreditAttribution: jasonrichardsmith@gmail.com commentedchanged proposed solution
Comment #20
c960657 CreditAttribution: c960657 commentedTested and reviewed.
I wonder whether we should skip the
|| location.hash == '#openid-login'
part from the D6 patch? It's a new feature that might surprise someone.Comment #20.0
c960657 CreditAttribution: c960657 commentedmoved original post