Export Bookmarks to html wrong with utf8

VERIFIED FIXED in Firefox 30

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: priston910423, Assigned: mak)

Tracking

({regression})

30 Branch
Firefox 32
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +

Firefox Tracking Flags

(firefox29 unaffected, firefox30+ fixed, firefox31+ fixed, firefox32+ fixed)

Details

(Whiteboard: [diamond] [good-next-bug] p=3 s=it-32c-31a-30b.2 [qa-])

Attachments

(1 attachment)

Reporter

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.43 BIDUBrowser/6.x Safari/537.31

Steps to reproduce:

Export Bookmarks to html


Actual results:

my bookmark address is
http://bt.ktxp.com/search.php?keyword=%E5%A6%84%E6%83%B3%E5%AD%A6%E7%94%9F%E4%BC%9A

it change
<A HREF="http://bt.ktxp.com/search.php?keyword=%25E5%25A6%2584%25E6%2583%25B3%25E5%25AD%25A6%25E7%2594%259F%25E4%25BC%259A" LAST_MODIFIED="1396869229" ICON_URI="http://static.ktxp.com/favicon.ico" ICON="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAAyElEQVQ4jaWS0Q3DIAwF3yAMkkXYAltiEBZhAIQ9AINkEfoBhbRK0jRBMlIUOO5hAJLyqNp0c+wCKBk4taBk7gEAgLXCib8PICmgtEwj8SApDaz2GsCpbRtzgBM/IIcGlAwoh3lSjzBAG6tTA0oLWOtYzBpBaQHJCsrhAqAbkKxw4sFam8mvCO9FrHFsohxGscZzA0qmaSbTI7TvbaxtB3YAs22stWc3HVS6Rf2w2L0Dp3ZG+HqV7V8Eazzrgrn0Cg8B/4wBeFAvPGD3uX8X93AAAAAASUVORK5CYII=">关键字 &quot;妄想学生会&quot; 的搜索结果 - 动漫下载|BT|漫画|动画|游戏 - 极影动漫</A>

in the html



Expected results:

I hava to many 25

IT only happened in 31.0a1

29 doesn't have this problem
Reporter

Updated

5 years ago
Component: Untriaged → Bookmarks & History
Assignee

Comment 1

5 years ago
sounds like a regression in the code generating the html
Blocks: 968177
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: firefox-backlog?
Keywords: regression
Assignee

Updated

5 years ago
OS: Windows 7 → All
Hardware: x86 → All
Flags: firefox-backlog? → firefox-backlog+
Reporter

Updated

5 years ago
Version: 31 Branch → 30 Branch
Reporter

Comment 2

5 years ago
it also happen in 30(beta)
With a decent regression range, I'm calling this a [diamond][good-next-bug].
Whiteboard: [diamond] [good-next-bug]
Assignee

Comment 4

5 years ago
(In reply to Mike Hoye [:mhoye] from comment #3)
> With a decent regression range, I'm calling this a [diamond][good-next-bug].

I don't think there's need of a more precise regrange, this has surely been caused by bug 968177, that is already being blocked here.
Whiteboard: [diamond] [good-next-bug] → [diamond] [good-next-bug] p=3
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Whiteboard: [diamond] [good-next-bug] p=3 → [diamond] [good-next-bug] p=3 s=it-32c-31a-30b.2 [qa?]
Whiteboard: [diamond] [good-next-bug] p=3 s=it-32c-31a-30b.2 [qa?] → [diamond] [good-next-bug] p=3 s=it-32c-31a-30b.2 [qa-]
We're in our fourth week of FF30 Beta - at this stage backouts to known good state are preferred if that's an option here, or a very low risk forward fix. Can you give an update on your progress here?
Flags: needinfo?(mak77)
Assignee

Comment 6

5 years ago
(In reply to Lukas Blakk [:lsblakk] from comment #5)
> We're in our fourth week of FF30 Beta - at this stage backouts to known good
> state are preferred if that's an option here, or a very low risk forward
> fix. Can you give an update on your progress here?

I'll work on this today, so I will have an update (hopefully a patch) before EOD, otherwise I will check if a backout would be safe (I'm a little scared of backing out, since a lot of code changed here)
Flags: needinfo?(mak77)
Reporter

Comment 7

5 years ago
(In reply to Lukas Blakk [:lsblakk] from comment #5)
> We're in our fourth week of FF30 Beta - at this stage backouts to known good
> state are preferred if that's an option here, or a very low risk forward
> fix. Can you give an update on your progress here?


I use the newest firefox 30(BETA) and it's still happen
Assignee

Comment 8

5 years ago
Posted patch patch v1Splinter Review
This was my fault, I mis-changed the behavior using encodeURI, when the old code was just escaping some url. our URIs are already encoded so we ended up double encoding.
This patch restores the original behavior.

It's a safe patch, very trivial and with a dedicated test, should be good for late beta.
Attachment #8425588 - Flags: review?(mano)
Comment on attachment 8425588 [details] [diff] [review]
patch v1

r=mano for this partial back-out. I don't fully understand why this code path need stripped-down escaping, but this is as good as it was.
Attachment #8425588 - Flags: review?(mano) → review+
Assignee

Comment 10

5 years ago
Yes, as you suggested I verified also Cu.nsINetUtil.ESCAPE_URL_MINIMAL would do, and would be more proper, but it's late to make experiments, and experimenting sounds more expensive than the benefit for this low priority code. Let's just go back to legacy code that we are sure worked as expected for years.
Assignee

Comment 11

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/b2c1d53130ff
Flags: in-testsuite+
Target Milestone: --- → Firefox 32
https://hg.mozilla.org/mozilla-central/rev/b2c1d53130ff
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Assignee

Comment 13

5 years ago
Comment on attachment 8425588 [details] [diff] [review]
patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 997030
User impact if declined: uris may be double encoded in bookmarks.html, causing nonworking uris on re-import
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk (partial backout of the change that caused this bug)
String or IDL/UUID changes made by this patch: none
Attachment #8425588 - Flags: approval-mozilla-beta?
Attachment #8425588 - Flags: approval-mozilla-aurora?
Attachment #8425588 - Flags: approval-mozilla-beta?
Attachment #8425588 - Flags: approval-mozilla-beta+
Attachment #8425588 - Flags: approval-mozilla-aurora?
Attachment #8425588 - Flags: approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.