Closed Bug 245758 Opened 20 years ago Closed 20 years ago

Persisting bookmarks file converts ' to ' in bookmark title (apostrophe, lsquo, entity)

Categories

(SeaMonkey :: Bookmarks & History, defect)

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: isaachh, Assigned: BenB)

References

(
URL
)

Details

(Keywords: fixed1.7.5, regression)

Attachments

(1 file, 3 obsolete files)

Fix, v3 - per jst's comment
782 bytes, patch
neil
: review+
mkaply
: approval1.7.5+
Details | Diff | Splinter Review 
1. Go to http://www.tomshardware.com/
2. Bookmark the page (Ctrl+D)
3. Restart mozilla (to force saving bookmark file)
4. See the bookmark title

Actual Result:   Tom‘s Hardware Guide
Expected Result: Tom's Hardware Guide
OK:     2004-06-05-10-trunk
Broken: 2004-06-06-10-trunk

Bug 245399 may have caused this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Depends on: 245399
No longer depends on: 245399
OS: Windows XP → All
Hardware: PC → All
Summary: Persisting bookmarks file converts ' to ‘ in bookmark title → Persisting bookmarks file converts ' to ‘ in bookmark title
-> me
Assignee: p_ch → ben.bucksch
Attached patch Fix, v1 (obsolete) — — Splinter Review 
Indeed a regression caused by bug 245399.

This is a trivial patch which fixes the problem for me.

See comments in bug 245399 wrt creating a Mozilla-wide unescape function.

None of this fixes bookmark descriptions already broken in profiles of nightly
users, and there's hardly anything we could do about it, sorry. Hopefully not
many such bookmarks exist.
Attachment #150175 - Flags: review?(p_ch)
Attached patch Fix, v2 (obsolete) — — Splinter Review 
New patch matching patch v3 in bug 245399.
Attachment #150179 - Flags: review?(p_ch)
*** Bug 245856 has been marked as a duplicate of this bug. ***
I've found that by overwriting my bookmark file with a previously saved bookmark
file, the problem goes away.  One build must have made the conversion, but with
current builds the quote is interpreted correctly.
*** Bug 245928 has been marked as a duplicate of this bug. ***
I'm running a 6/8-09 build of Mozilla under XP.  For me, I get ' replaced by '
I got ' too, on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2)
Gecko/20040608 Firefox/0.8.0+, when I open and close Firefox with all
&‘ replaced with ' in bookmarks.html.
Attachment #150175 - Attachment is obsolete: true
Attachment #150179 - Attachment description: Fix, v2 - only, if 245399 gets changed again → Fix, v2
Attachment #150179 - Flags: superreview?(jst)
Status: NEW → ASSIGNED
Summary: Persisting bookmarks file converts ' to ‘ in bookmark title → Persisting bookmarks file converts ' to ' in bookmark title
Attachment #150175 - Flags: review?(p_ch)
*** Bug 246040 has been marked as a duplicate of this bug. ***
*** Bug 246045 has been marked as a duplicate of this bug. ***
*** Bug 246053 has been marked as a duplicate of this bug. ***
Tweaking title to try and stem the number of dups.
Summary: Persisting bookmarks file converts ' to ' in bookmark title → Persisting bookmarks file converts ' (apostrophe) to ' (lsquo) (entity) in bookmark title
Summary: Persisting bookmarks file converts ' (apostrophe) to ' (lsquo) (entity) in bookmark title → Persisting bookmarks file converts ' to ' in bookmark title (apostrophe, lsquo, entity)
Comment on attachment 150179 [details] [diff] [review]
Fix, v2

+	 else if (Substring(text, offset, 5).Equals(NS_LITERAL_STRING("'"),
nsCaseInsensitiveStringComparator()))

Nothing case-dependent in that string, no need to pass in
nsCaseInsensitiveStringComparator().

sr=jst
Attachment #150179 - Flags: superreview?(jst) → superreview+
isn't there an EqualsLiteral these days?
Attachment #150179 - Attachment is obsolete: true
Attachment #150429 - Attachment description: Fix, v3 - fixes jst's comment → Fix, v3 - per jst's comment
Attachment #150429 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 150179 [details] [diff] [review]
Fix, v2

r=me if you use EqualsLiteral or at least warn roc
Attachment #150179 - Flags: review?(p_ch) → review+
Thanks. Checked v3 in.

ccing roc. This will cause a conflict for him anyways, no matter what I use.
Sorry for that, but I can't leave this bug any longer.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment on attachment 150429 [details] [diff] [review]
Fix, v3 - per jst's comment

I still think roc would want to improve on this.
Attachment #150429 - Flags: review?(neil.parkwaycc.co.uk) → review+
*** Bug 246202 has been marked as a duplicate of this bug. ***
*** Bug 246231 has been marked as a duplicate of this bug. ***
*** Bug 246304 has been marked as a duplicate of this bug. ***
*** Bug 246384 has been marked as a duplicate of this bug. ***
Reopening since I have this patch and my 's are still being munged.  I notice
that in my bookmark file it's actually storing a '.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Please try creating a *new* bookmark <http://www.tomshardware.com> or using a
bookmarks file that were never touched by the broken builds.
I never used one of the builds listed as broken and this happens with new bookmarks.

I start with a ' in my bookmark file.  Upon loading it changes the ' to a &#39;
and displays correctly.  Upon loading after that it changes the &#39; to a
&amp;#39; and displays incorrectly.
Attached patch Fix Firefox bookmarks (obsolete) — — Splinter Review 
Since people are dup'ing firefox bugs to this one...
Attachment #150567 - Flags: review?(mconnor)
Gyre, I can't reproduce. I bookmark Tom's Hardware, close, start, close, start,
bookmark is correct. Please make sure to use a nightly build *with* the checkin.

> I have this patch

Are you using your own build? You'll need all the checkins for bug 245399, too.

swalker, I'd prefer if you used a new bug for Firefox.
Re: Comment #27
I know it's a long shot, but what if people actually want a &#39; in there? Some
pages have a crazy title. Say someone were to bookmark this very page, wouldn't
the title read "... converts ' to ' in bookmark..." then?
> what if people actually want a &#39; in [the title]?

We will escape "&" to "&amp;" during saving and will unescape "&amp;" during
load, ending up with exactly what you added. (Assuming no bugs.) That's one of
the points of proper escaping.
(In reply to comment #28)
> swalker, I'd prefer if you used a new bug for Firefox.

Bug 246419 has been opened on the Firefox side.  The patch should be moved over
to that bug.
Argh, I need to pay more attention to the component.  I filed a bug on FF which
got duped here, and obviously that's why I'm not seeing this fix.

Feel free to reresolve if you're splitting into separate bugs.
(In reply to comment #32)
> Argh, I need to pay more attention to the component.  I filed a bug on FF which
> got duped here, and obviously that's why I'm not seeing this fix.
> 
> Feel free to reresolve if you're splitting into separate bugs.

I think everyone (including me) was just assuming that one fix would cover both,
not knowing the code would have to be ported.  Thus the Firefox bugs were duped
here.

Resolving this bug again per last comment.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Attachment #150567 - Attachment is obsolete: true
Attachment #150567 - Flags: review?(mconnor)
*** Bug 246585 has been marked as a duplicate of this bug. ***
*** Bug 247924 has been marked as a duplicate of this bug. ***
*** Bug 248757 has been marked as a duplicate of this bug. ***
This bug is still NOT fixed.

I added a new bookmark of www.aintitcoolnews.com (which has Ain't It Cool News
as the title), and it shows up as:

Ain&#39;t It Cool News
What build are you using?

WFM, with build 2004062608 on Mac OS X.
Build 2004-07-05-09, Windows XP.
I have no problem with Tom's Hardware (it shows up correctly when I bookmark it,
close Mozilla, restart and look at the bookmark manager).

But, I do have the problem with Ain't it Cool News, so perhaps I should file a
new, separate bug.
The bug still exists in Mozilla 1.8 Alpha 2 on Linux.
*** Bug 249395 has been marked as a duplicate of this bug. ***
(In reply to comment #41)
> The bug still exists in Mozilla 1.8 Alpha 2 on Linux.

You sure !?

My tests on Windows:

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040714] (release) (W98SE)

Converts "'" to "&#39;", as intended, as I understand !?
And deconverts it properly on displaying.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a1) Gecko/20040520] (release) (W98SE)
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.1) Gecko/20040707] (release) (W98SE)

They don't convert "'" to "&#39;" :-|
But they convert "&#39;" to "&amp;#39;" :-(

Then,
v1.8a2 wouldn't be broken;
but previous releases are now incompatable !!?

Ben, is that it ?
If yes, shouldn't there be a warning in release notes, or something ??
*** Bug 251951 has been marked as a duplicate of this bug. ***
All my existing bookmarks with apostrophs (') were broken by this bug.
The fix didn't reverse the breakage, but new bookmarks are OK.
I have to edit the old ones manually to fix them.
I assume this is known and the expected behaviour after the fix?
Yes, see http://bugzilla.mozilla.org/show_bug.cgi?id=245758#c3

Also, if you run an older build (before the fix) at all, even with a profile
that had originally written out bookmarks correctly, the bookmarks will again be
'broken'.
*** Bug 252031 has been marked as a duplicate of this bug. ***
Attachment #150429 - Flags: approval1.7.3?
Comment on attachment 150429 [details] [diff] [review]
Fix, v3 - per jst's comment

a=mkaply for 1.7.3
Attachment #150429 - Flags: approval1.7.3? → approval1.7.3+
Re: Comment #46

So, does that mean if I use Mozilla 1.7 and 1.8a3 alternately, this bug will
re-surface again? Because it is what happens in my situation, using Linux Mozilla.
Yes, actually, it probably gets worse every time you switch. It should be solved
for 1.7.3, if the fix gets checked into the 1.7 branch (we have approval, but I
don't care anymore).
Thanks, that clears it up.
*** Bug 256193 has been marked as a duplicate of this bug. ***
*** Bug 257928 has been marked as a duplicate of this bug. ***
*** Bug 259022 has been marked as a duplicate of this bug. ***
Re: Comment #50

Just tested Mozilla 1.7.3 today, well the changeover issue when switching
between Mozilla 1.8 alpha3 and 1.7.x still persists. 

Perhaps I'll wait a bit before moving to 1.8 later, else I would have to leave
Mozilla 1.7 altogether because of this incompatible convert issue between them.
*** Bug 259373 has been marked as a duplicate of this bug. ***
*** Bug 264541 has been marked as a duplicate of this bug. ***
Keywords: fixed1.7.x
Just use mozilla 1.8a3 or a4, it doesn't suffer from this bug.
bye!
steer clear from 1.7.3 and/or NS 7.2.
Product: Browser → Seamonkey
*** Bug 271464 has been marked as a duplicate of this bug. ***
*** Bug 271464 has been marked as a duplicate of this bug. ***
*** Bug 281934 has been marked as a duplicate of this bug. ***
*** Bug 291249 has been marked as a duplicate of this bug. ***
*** Bug 297981 has been marked as a duplicate of this bug. ***
Gentlefolk:
   Doesn't anyone find it a mite strange that a "RESOLVED FIXED" bug is getting
so many supposed duplicates after it was fixed.  This appears to be still present.

QA: Please consider re-opening this or letting, for example, my Bug 297981 be
activew.
*** Bug 341039 has been marked as a duplicate of this bug. ***
I agree with comment #64.  It happened to me with Seamonkey 1.0.2, the most recent version (and because I couldn't find this or any of the dupes in the 200 results the search function found me, I submitted it as bug #341039).  Some of the instances mentioned below may be fixed, but others are not.

I don't know whether my previous browser (Netscape 7.2) stored them as ' or as &#39;.  If it stored them as &#39;, then it was at least smart enough to unescape it for display purposes, and Seamonkey isn't (so that ought to be fixed).  If it stored them as ', then somehow Seamonkey broke them, and that ought to be fixed (along with fixing its inability to unescape these things for display).

Either way, there's something that ought to be fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: