Reported in Google Webmaster Tools: a lot of crawl errors were introduced by mydomain.com/[path here]%2523 links.

The following code in openid.module is causing the issue:
Line 104

l(t('Log in using OpenID'), '#');

Line 108

l(t('Cancel OpenID login'), '#');

which returns <a href="/%2523">Cancel OpenID login</a>

Reproduce: hover over Log in using OpenID at http://www.badexample.com/ at notice http://www.badexample.com/%2523

CommentFileSizeAuthor
#20 openid-fragment-5.patch1.06 KBtoemaz
#18 openid-fragment-5.patch1.01 KBtoemaz
#14 openid-fragment-4.patch1.01 KBtoemaz
#12 openid-fragment-4.patch1.04 KBtoemaz
#10 openid-fragment-3.patch1.01 KBtoemaz
#7 openid-fragment-2.patch1.01 KBtoemaz
#3 openid-fragment-2.patch1.04 KBtoemaz
#1 openid-fragment.patch779 bytestoemaz

Comments

toemaz’s picture

toemaz commented
Version: 8.x-dev » 7.x-dev
Status: Active » Needs review
StatusFileSize
new openid-fragment.patch779 bytes

Patch created against D7-UNSTABLE10 based on answer by googletorp.

alexanderpas’s picture

alexanderpas commented 
+++ openid.module	19 Dec 2009 13:57:01 -0000
@@ -98,11 +98,11 @@
+    'data' => l(t('Log in using OpenID'), '', array('fragment' => ' ', 'external' => TRUE)),

Could we document here why exactly we're doing this.

This review is powered by Dreditor.

toemaz’s picture

toemaz commented
StatusFileSize
new openid-fragment-2.patch1.04 KB

Attached is a new patch with documentation.

BTW: check Google Webmaster Tools for your website http://dropdev.org . You will also notice the crawl errors pointing to %2523, which are introduced by the openid login links. Far from good to obtain a good search engine ranking.

Status: Needs review » Needs work

The last submitted patch, , failed testing.

Status: Needs work » Needs review

Re-test of from comment #2393094 was requested by @user.

Status: Needs review » Needs work

The last submitted patch, , failed testing.

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-2.patch1.01 KB

Unix formatted patch.

Status: Needs review » Needs work

The last submitted patch, openid-fragment-2.patch, failed testing.

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-3.patch1.01 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, openid-fragment-3.patch, failed testing.

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-4.patch1.04 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, openid-fragment-4.patch, failed testing.

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-4.patch1.01 KB

Unix formatting

Status: Needs review » Needs work

The last submitted patch, openid-fragment-4.patch, failed testing.

toemaz’s picture

toemaz commented

Clearly I miss something to create a good working patch. I guess I need a hint.

asimmonds’s picture

asimmonds commented

You need to roll the patch from Drupal's root directory. ie where index.php is located.

See the CVS example in http://drupal.org/node/323

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-5.patch1.01 KB

Thx asimmonds. New patch attached.

toemaz’s picture

toemaz commented
Status: Needs review » Needs work

Still not ok. I followed the steps for how to make a patch with Tortoise on Windows, but apparently this patch is simply the same as the previous ones. However, this time I made the patch from the Drupal root.

toemaz’s picture

toemaz commented
Status: Needs work » Needs review
StatusFileSize
new openid-fragment-5.patch1.06 KB

Made with Eclipse this time since TortoiseCVS was no use.

toemaz’s picture

toemaz commented

Anyone willing to review this? I have this patch running on several production websites. If not and openid enabled, watchdog is filled with page not found warnings

Type	page not found
Date	Thursday, January 7, 2010 - 12:58
User	Anonymous
Location	http://www.mysite.com/%2523
Referrer	
Message	#
Severity	warning
Hostname	119.63.198.122
danepowell’s picture

danepowell commented

Sorry I can't test it on D7, but it does seem to work on D6. I'm incorporating it into the CAS module, which borrowed a lot of code from OpenID. Thanks!

toemaz’s picture

toemaz commented

Thanks for the thumbs up. It's kinda strange that no one reacts on this issue. Even the recently launched http://opensource.com has this issue. Let them look to their google webmaster tools and they'll see it popping up.

c960657’s picture

c960657 commented

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.

c960657’s picture

c960657 commented
Status: Needs review » Closed (duplicate)

I just realized that this is a duplicate of #160163: Login with OpenID link has mis-encoding of #.

srobert72’s picture

srobert72 commented

Subscribing
Patch #20 works for me

sergmain’s picture

sergmain commented

subscribe