Closed Bug 371109 Opened 14 years ago Closed 14 years ago

Sending URL-encoded links when outgoing color codes enabled

Categories

(Other Applications :: ChatZilla, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: vr, Assigned: Mook)

Details

(Whiteboard: [cz-0.9.78])

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Chatzilla 0.9.77 [Firefox 2.0.0.1/2006120418]

Chatzilla breaks URL-encoded links, treating some of the symbols as formatting characters (for example, %B7 becomes bold 7).

Reproducible: Always

Steps to Reproduce:
1. Send http://ru.wikipedia.org/wiki/%D0%A7%D0%B0%D1%82%D0%B7%D0%B8%D0%BB%D0%BB%D0%B0 to the chat window.

Actual Results:  
Link appears in chat window as http://ru.wikipedia.org/wiki/%D0%A7%D00%D1%82%D07%D08%D0B%D0B%D00

Expected Results:  
Link should not be changed and has to appear as http://ru.wikipedia.org/wiki/%D0%A7%D0%B0%D1%82%D0%B7%D0%B8%D0%BB%D0%BB%D0%B0
ChatZilla treats the %-codes (only some letters, as you know) as formatting only if you turn it on. It's not possible to have it both ways, leaving this bug as INVALID.
Leaving this as INVALID will make some users leave Chatzilla for alternatives. For example, mIRC is smart enough to handle such URLs without the need to turn off formatting support.
Attached patch workaroundSplinter Review
This works around the colorizing by escaping things which look like URLs:

If some string contains "://" followed by non-whitespace followed by "%" followed by two hex digits (and that "%" was not prefixed by another "%"), then escape the "%".  This will not catch manually entered URL-like things without the scheme ("www.mozilla.org"), but they're also less likely to be URL-escaped.  (This does not catch non-URL URIs either - "rdf:bookmarks" - but I don't think we care.)

Not sure if this is the right way to go, but does seem to work on some random samples (I used devmo's Japaneses main page).  I believe there will be objections, but I'd at least like specific reasons on why this is a bad idea :)
Attachment #256003 - Flags: review?(silver)
(In reply to comment #2)

I love it when users have no clue what they're talking about.
Mook, do you think it'd be possible to utilise client.linkRE instead of having another link-related regexp to maintain?

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/xul/lib/munger.js&rev=1.23#43
Severity: normal → enhancement
Summary: Sending URL-encoded links is broken → Sending URL-encoded links when outgoing color codes enabled
Version: unspecified → Trunk
Use client.linkRE instead.  This means it matches the exact same set that normal munger does - www.example.com/%B0 will get matched this time around (it didn't with the last patch).  This means you can't manually highlight things either - e.g. "www.getfire%Bb%Oox.com".  (Hmm, should we only match upper case instead?)

Also, I had to change linkRE to use a lookahead on the last separator in order to not skip every other URL.  If that is not desirable, I can probably spin up a patch that works around that instead.
Attachment #256026 - Flags: review?(silver)
I wonder who will be the first person to complain that they can't put colors in their links? ;-)  Especially since we now support that...
Comment on attachment 256026 [details] [diff] [review]
use client.linkRE

We can't have everything. Bold still works in basic cases, like www.%Bmozilla%O.com, and there's little we can do right now about the ambiguous cases in URLs.

r=silver, though note the client.linkRE line has moved to xul/content/munger.js now. No other change needed.
Attachment #256026 - Flags: review?(silver) → review+
Attachment #256003 - Flags: review?(silver)
Assignee: rginda → mook.moz+mozbz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Checking in mozilla/extensions/irc/xul/content/mungers.js;
/cvsroot/mozilla/extensions/irc/xul/content/mungers.js,v  <--  mungers.js
new revision: 1.3; previous revision: 1.2
done
Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.233; previous revision: 1.232
done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.78]
You need to log in before you can comment on or make changes to this bug.