Closed
Bug 453194
Opened 16 years ago
Closed 15 years ago
Refactor Safari and Opera import/export into HTMLBookmarkConverter-style classes
Categories
(Camino Graveyard :: Bookmarks, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Camino2.0
People
(Reporter: stuart.morgan+bugzilla, Assigned: stuart.morgan+bugzilla)
Details
(Whiteboard: [camino-2.0])
Attachments
(1 file)
81.06 KB,
patch
|
mikepinkerton
:
superreview+
|
Details | Diff | Splinter Review |
All the bookmark import/export should be visitor-pattern-based, so the format details aren't wound through the various bookmark classes.
Updated•16 years ago
|
Hardware: PC → All
Assignee | ||
Comment 1•15 years ago
|
||
This moves all the import/export for Safari and Opera bookmarks into their own classes, rather than having them scattered through the bookmark system, and restructures the Opera import to be more understandable and have less duplicate code. In addition, a couple of related incidentals: - Added support for Opera 10 (which has a new extension on the bookmarks file) - Remove an encoding-guessing function which was no longer being used (since the HTML import rewrite, it was only ever used to guess that Opera files are UTF8, which we don't need to guess) - Remove a loading function that would read a Safari plist as the actual bookmark backing store if someone put it in place of the Camino bookmarks file. That's completely undiscoverable, and I couldn't think of any good reason to support it. - Tweaks the HTML import/export class slightly for consistency of return values with the new import/export classes.
Attachment #404501 -
Flags: superreview?(mikepinkerton)
(In reply to comment #1) > - Remove a loading function that would read a Safari plist as the actual > bookmark backing store if someone put it in place of the Camino bookmarks file. > That's completely undiscoverable, and I couldn't think of any good reason to > support it. As I recall, we added that because people were doing it. Please don't ask me to explain *why* our users were doing it, but they were.
Assignee | ||
Comment 3•15 years ago
|
||
Unfortunately it was part of a big smfr checkin with no associated bug, so I can't find any discussion. My feeling is that we should take the code out, and then people will stop doing it ;) If people feel strongly that we should still allow that, I'll have to update the patch with a modified version of that code.
Comment 4•15 years ago
|
||
Comment on attachment 404501 [details] [diff] [review] refactoring sr=pink
Attachment #404501 -
Flags: superreview?(mikepinkerton) → superreview+
Assignee | ||
Comment 5•15 years ago
|
||
Landed on CVS trunk and CAMINO_2_0_BRANCH.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [camino-2.0]
Target Milestone: --- → Camino2.0
You need to log in
before you can comment on or make changes to this bug.
Description
•