browser.bookmarks.autoExportHTML did not work anymore

VERIFIED FIXED in Firefox 26

Status

()

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: fxtech, Assigned: marco)

Tracking

({regression})

26 Branch
mozilla27
x86
All
regression
Points:
---

Firefox Tracking Flags

(firefox25 unaffected, firefox26+ verified, firefox27+ verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20130921030221

Steps to reproduce:

set browser.bookmarks.autoExportHTML to true , it should create a file called bookmarks.html under the firefox profile path , usually under %appdata%\mozilla\firefox\profiles\



Actual results:

The problem happens on Nightly 27.0a1 and Aurora26.0a2, but not on Beta25.0b1

The file is not created and an empty directory called \bookmark.html\ is created instead


Expected results:

instead of a directory a file called bookmarks.html should be created
(Reporter)

Updated

5 years ago
Hardware: x86_64 → x86
(Reporter)

Updated

5 years ago
Component: Untriaged → Bookmarks & History

Comment 1

5 years ago
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/4ccfffb6262a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130830171559
Bad:
http://hg.mozilla.org/mozilla-central/rev/1ac8270f3f64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130830173759
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4ccfffb6262a&tochange=1ac8270f3f64



Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d4141cb0d9cf
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130830131457
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/6bd785bca4ff
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130830131557
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d4141cb0d9cf&tochange=6bd785bca4ff

Regressed by:
6bd785bca4ff	Marco Castelluccio — Bug 910885 - Improve FileUtils.getDir(..., ..., true) performance. r=Yoric
Blocks: 910885
Status: UNCONFIRMED → NEW
status-firefox25: --- → unaffected
tracking-firefox26: --- → ?
tracking-firefox27: --- → ?
Ever confirmed: true
Keywords: regression

Comment 2

5 years ago
I can also reproduce on ubuntu 12.04.
http://hg.mozilla.org/mozilla-central/rev/8b4d14afc4f6
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20130922030201
OS: Windows 7 → All

Updated

5 years ago
Component: Bookmarks & History → General
Product: Firefox → Toolkit

Updated

5 years ago
Version: 27 Branch → 26 Branch
Marco, do we need to backout bug 910885 or do you have a potential forward fix?  We'd need to resolve this on Aurora before shipping to a larger audience.
Assignee: nobody → mcastelluccio
status-firefox26: --- → affected
status-firefox27: --- → affected
tracking-firefox26: ? → +
tracking-firefox27: ? → +
Created attachment 809306 [details] [diff] [review]
fix_bookmarks_export

I'm not sure who should review this patch.

Comment 5

5 years ago
Comment on attachment 809306 [details] [diff] [review]
fix_bookmarks_export

mak seems like a reasonable choice.
Attachment #809306 - Flags: review?(mak77)
Care to explain what changed from before? looks like the only exception is handled properly in getDir, so why is this not working anymore?
There's a subtle difference.

FileUtils.getFile uses FileUtils.getDir to get the parent directory of the file.

Before bug 910885, in FileUtils.getDir we didn't create the directory if a file with the same name of the directory existed.
So if a file "bookmarks.html" had existed, getDir wouldn't have created a directory with the "bookmarks.html" name. Now it's no longer the case, because getDir doesn't check if the file exists before creating the directory.
(In reply to Marco Castelluccio [:marco] from comment #7)
> So if a file "bookmarks.html" had existed, getDir wouldn't have created a
> directory with the "bookmarks.html" name. Now it's no longer the case,
> because getDir doesn't check if the file exists before creating the
> directory.

Ah, I see what you mean.
Basically getFile expects the key to be a directory, but this is not the case for various dirsvc keys...
To me this sounds like poor behavior of FileUtils, but since Yoric just filed bugs to deprecate getFile and getDir, I guess we don't care to fix it now. I'm surprised we implemented a so error-prone helper, you call .getFile(), and it creates a folder with the name of your file :(
Comment on attachment 809306 [details] [diff] [review]
fix_bookmarks_export

Review of attachment 809306 [details] [diff] [review]:
-----------------------------------------------------------------

please ask for approval once this lands, the fix is quite safe
Attachment #809306 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #8)
> (In reply to Marco Castelluccio [:marco] from comment #7)
> > So if a file "bookmarks.html" had existed, getDir wouldn't have created a
> > directory with the "bookmarks.html" name. Now it's no longer the case,
> > because getDir doesn't check if the file exists before creating the
> > directory.
> 
> Ah, I see what you mean.
> Basically getFile expects the key to be a directory, but this is not the
> case for various dirsvc keys...
> To me this sounds like poor behavior of FileUtils, but since Yoric just
> filed bugs to deprecate getFile and getDir, I guess we don't care to fix it
> now. I'm surprised we implemented a so error-prone helper, you call
> .getFile(), and it creates a folder with the name of your file :(

Yes, that's a very weird behavior. Luckily looks like the other keys that reference files aren't being used with getFile.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/181b43d77162
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/181b43d77162
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox27: affected → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla27
Comment on attachment 809306 [details] [diff] [review]
fix_bookmarks_export

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 910885.
User impact if declined: Users wouldn't be able to export bookmarks through the browser.bookmarks.autoExportHTML property.
Testing completed (on m-c, etc.): Tested locally, but it's a really simple fix.
Risk to taking this patch (and alternatives if risky): Very low risk.
String or IDL/UUID changes made by this patch: None.
Attachment #809306 - Flags: approval-mozilla-aurora?
Attachment #809306 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm receiving Release Tracking emails, but this bug has landed.

Comment 16

5 years ago
Verified on Aurora 26.0a2 buildid 20131011004001, Nightly 27.0a1 buildid 20131010030202.
QA Contact: ananuti

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox26: fixed → verified
status-firefox27: fixed → verified
You need to log in before you can comment on or make changes to this bug.