Can't copy the multibyte character correctly from the location bar

VERIFIED FIXED in Firefox 8

Status

()

Core
Internationalization
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

(6 keywords)

unspecified
mozilla10
x86_64
Windows 7
dev-doc-complete, intl, jp-critical, regression, verified-aurora, verified-beta
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox8+ fixed, firefox9 fixed)

Details

(Whiteboard: [qa!], URL)

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

6 years ago
Steps to reproduce:
1. Open the URL.
2. Copy the URL from the location bar.

Actual result:
https://encrypted.google.com/search?q=%E3%81%82&ie=utf-8

Expected result:
https://encrypted.google.com/search?q=%82%A0&ie=utf-8

It works on Firefox 7 release, broken since Firefox 8.0b1.

Comment 1

6 years ago
Expected Result:
https://encrypted.google.com/search?q=%E3%81%82&ie=utf-8
Actual Result:
https://encrypted.google.com/search?q=%82%A0&ie=utf-8

Result when pasting URL to other application:
https://encrypted.google.com/search?q=%82%A0&ie=utf-8

This problem seems to be problem when copying URL to clipboard.

Is this bug related to bug 668019 ?
Requesting tracking for fx8.

A regression range would likely answer the question from comment 1....
Blocks: 668019
tracking-firefox8: --- → ?
(Assignee)

Comment 3

6 years ago
Regression window (mozilla-aurora)
Works: http://hg.mozilla.org/releases/mozilla-aurora/rev/009c64b64cf3
Broken: http://hg.mozilla.org/releases/mozilla-aurora/rev/c440edd84f84
Pushlog: https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=009c64b64cf3&tochange=c440edd84f84
It's most likely to be bug 668019.
Keywords: regressionwindow-wanted
(Assignee)

Comment 4

6 years ago
I propose add an option to CreateFixupURI so that it encodes URI with UTF-8 instead of platform charset (and backout bug 668019 from aurora and beta).
I can't reproduce. Maybe this only occurs when the default encoding is Shift_JIS?
(Assignee)

Comment 6

6 years ago
(In reply to Simon Montagu from comment #5)
> I can't reproduce. Maybe this only occurs when the default encoding is
> Shift_JIS?
Yes. It occurs only when the platform charset is not UTF-8 and the character has a mapping on the platform charset. ISO-8859-1 doesn't have a map of "あ", so it will fallback to UTF-8.
(Assignee)

Comment 7

6 years ago
It should be reproducible with <https://encrypted.google.com/search?q=÷&ie=utf-8> on Latin-1 system locale.
(Assignee)

Comment 8

6 years ago
Steps to reproduce:
1. Open <https://encrypted.google.com/search?q=%C3%B7&ie=utf-8>.
2. Copy the URL from the location bar.

Actual result:
https://encrypted.google.com/search?q=%81%80&ie=utf-8 (on Shift_JIS locale)
https://encrypted.google.com/search?q=%F7&ie=utf-8 (on Latin-1 locale)

Expected result:
https://encrypted.google.com/search?q=%C3%B7&ie=utf-8
This was broken before bug 668019 in the sense that not copying but just hitting enter would mess up the encoding.
(Assignee)

Comment 10

6 years ago
(In reply to Dão Gottwald [:dao] from comment #9)
> This was broken before bug 668019 in the sense that not copying but just
> hitting enter would mess up the encoding.
I know. Things getting even worse since Firefox 8 because Japanese users can no longer even copy the URL correctly.
(Assignee)

Updated

6 years ago
See Also: → bug 461304
Keywords: jp-critical
(Assignee)

Comment 11

6 years ago
Created attachment 565431 [details] [diff] [review]
Part 1: add UTF-8 flag to createFixupURI
Assignee: smontagu → VYV03354
Status: NEW → ASSIGNED
Attachment #565431 - Flags: review?(bzbarsky)
(Assignee)

Comment 12

6 years ago
Created attachment 565432 [details] [diff] [review]
Part 2: Use UTF-8 flag on copy

We should not use platform charset here because the URI is always decoded in UTF-8 regardless of the platform charset on the location bar.
Attachment #565432 - Flags: review?(gavin.sharp)
Comment on attachment 565431 [details] [diff] [review]
Part 1: add UTF-8 flag to createFixupURI

I don't think you need to rev the IID for the flag addition; it's completely ABI and API compatible.

r=me without the iid rev.
Attachment #565431 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 14

6 years ago
Comment on attachment 565432 [details] [diff] [review]
Part 2: Use UTF-8 flag on copy

This patch causes misterious test timeout...

> TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_urlbarCopying.js | Expecting copy of: http://example.com/?%C3%B7%C3%B7
> TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/browser_urlbarCopying.js | Console message: Direct3D 9 DeviceManager Initialized Succesfully.
> Driver: igdumdx32.dll
> Description: Mobile Intel(R) 965 Express Chipset Family
> Version: 8.14.10.1930
> TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_urlbarCopying.js | Timed out while polling clipboard for pasted data.
> Stack trace:
>     JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: wait :: line 634
>     JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: <TOP_LEVEL> :: line 659
>     native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

But it seems to be unrelated to this fix. The timeout is caused the test change alone.
Attachment #565432 - Flags: review?(gavin.sharp)
(Assignee)

Comment 15

6 years ago
Created attachment 565549 [details] [diff] [review]
Backout bug 668019

Requesting backout on branches to workaround a critical regression.
Attachment #565549 - Flags: approval-mozilla-beta?
Attachment #565549 - Flags: approval-mozilla-aurora?
Comment on attachment 565549 [details] [diff] [review]
Backout bug 668019

We're not going to back out that patch - the bug it fixes affects more users than this bug does.
Attachment #565549 - Flags: review-
Attachment #565549 - Flags: approval-mozilla-beta?
Attachment #565549 - Flags: approval-mozilla-aurora?
(In reply to Masatoshi Kimura [:emk] from comment #14)
> But it seems to be unrelated to this fix. The timeout is caused the test
> change alone.

That's how failures in this test manifest themselves (the clipboard polling fails to find the right value on the clipboard, so it times out).

https://people.mozilla.com/~gavin/clipboardDebug.diff may be useful in debugging the issue.
I think we can take the actual fix on beta/aurora, to avoid shipping the regression.
(Assignee)

Comment 19

6 years ago
Created attachment 565708 [details] [diff] [review]
Part 1: Add UTF-8 flag to createFixupURI, v2

Ah, I overlooked "http://" case because the comment #0 URI was "https://".
Also canceled IID bumping. it's still ABI and API compatible.
Requesting review again because it contains an actual code change from the previous patch.
Attachment #565431 - Attachment is obsolete: true
Attachment #565708 - Flags: review?(bzbarsky)
(Assignee)

Comment 20

6 years ago
Comment on attachment 565432 [details] [diff] [review]
Part 2: Use UTF-8 flag on copy

This part didn't contain the bug.
Attachment #565432 - Flags: review?(gavin.sharp)
(Assignee)

Updated

6 years ago
Attachment #565549 - Attachment is obsolete: true
Attachment #565708 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 21

6 years ago
Gavin, ping?
(Assignee)

Comment 22

6 years ago
FYI, comment #14 problem has been resolved by Part 1 v2.
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=611d186aafad
Attachment #565432 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 23

6 years ago
Created attachment 567959 [details] [diff] [review]
565708: Part 1: Add UTF-8 flag to createFixupURI, v2

Un-bug690892-bitrotting.
Carrying forward r+.
Attachment #565708 - Attachment is obsolete: true
Attachment #567959 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
landed to m-i
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9ec8dc0fd61 (part 1)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d6fdd47ee1b (part 2)
status-firefox8: --- → affected
status-firefox9: --- → affected
Keywords: checkin-needed
Whiteboard: [inbound]

Updated

6 years ago
Flags: in-testsuite+
Comment on attachment 567959 [details] [diff] [review]
565708: Part 1: Add UTF-8 flag to createFixupURI, v2

requesting for aurora and beta since this is regression.
Attachment #567959 - Flags: approval-mozilla-beta?
Attachment #567959 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/c9ec8dc0fd61
https://hg.mozilla.org/mozilla-central/rev/6d6fdd47ee1b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Assignee)

Comment 27

6 years ago
Created attachment 568008 [details] [diff] [review]
Part 1 for aurora and beta

The trunk patch don't apply to branches cleanly because of PRBool->bool switch.
Part 2 is not affected.
Attachment #568008 - Flags: approval-mozilla-beta?
Attachment #568008 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

6 years ago
Attachment #567959 - Flags: approval-mozilla-beta?
Attachment #567959 - Flags: approval-mozilla-aurora?

Comment 28

6 years ago
Comment on attachment 568008 [details] [diff] [review]
Part 1 for aurora and beta

approving for aurora but to take this into beta, we'd like to get a risk analysis. What could go wrong. What should we be looking for in feedback if something does go wrong with this fix?
Attachment #568008 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 29

6 years ago
This patch changes a copy operation on the location bar. If something goes wrong, people will complain about wrong copied strings just like this bug. But I believe it's unlikely to occur.
Luckily, this patch avoids an IID bump. So it will not affect extension compatibility.

Comment 30

6 years ago
---------------------------------[ Triage Comment ]---------------------------------

Approving for 8beta as this was a regression in Firefox 7, the fix looks localized, and there is no add-on compatibility concern.

Please land ASAP.

Updated

6 years ago
Attachment #568008 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 31

6 years ago
---------------------------------[ Triage Comment ]---------------------------------

We'll track this for Firefox 8 as it is a regression and it would be good to track if the fix makes it in or not.
tracking-firefox8: ? → +
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [inbound] → land attachment #568008 and #565432 to aurora and beta
http://hg.mozilla.org/releases/mozilla-aurora/rev/dae0bfc9a760
http://hg.mozilla.org/releases/mozilla-aurora/rev/99db0336dcd1
http://hg.mozilla.org/releases/mozilla-beta/rev/ff142e5fea36
http://hg.mozilla.org/releases/mozilla-beta/rev/9e0f94a06cac
status-firefox8: affected → fixed
status-firefox9: affected → fixed
Keywords: checkin-needed
Whiteboard: land attachment #568008 and #565432 to aurora and beta
qa+ for verification in Firefox 8 and 9.
Whiteboard: [qa+]
(Assignee)

Comment 34

6 years ago
FIXUP_FLAG_USE_UTF8 needs to have a document.
Keywords: dev-doc-needed
What's the expected result? Comment 0 and comment 1 seems to be contradictory.
I've verified this on:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0.1) Gecko/20100101 Firefox/8.0.1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

and when pasting the URL to another application, the actual result is:
https://encrypted.google.com/search?q=%E3%81%82&ie=utf-8
(Assignee)

Comment 36

6 years ago
Sorry, comment #1 is correct.
Based on comment 35 and comment 36 marking the bug as verified fixed.

Updated

6 years ago
Status: RESOLVED → VERIFIED
Keywords: verified-aurora, verified-beta
Whiteboard: [qa+] → [qa!]
Documented (along with the rest of nsIURIFixup):

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIURIFixup

And mentioned on Firefox 10 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.