196756 - Meta bug for major architectural changes in bookmarks code

Status: RESOLVED INCOMPLETE

(SeaMonkey :: Bookmarks & History, defect)

Description

[Jan Varga [:janv]]

I know that there is bug 160019, but I want to keep this separate.
There are many issues which need major changes in the code, so it's not possible
to fix all those bugs separately. I created a new branch for this purpose.
The branch name is BOOKMARKS_20030310_BRANCH.
I'll add dependencies for bugs we plan to fix on the branch.

Comment 1

[Vaclav Dvorak]

How about bug 82721, then, which blocks bug 89001?

Comment 2

[Jan Varga [:janv]]

I also included some cleanup bugs and minor issues, sorry for the SPAM.

Comment 3

[Garth Wallace]

Bug #25909, bug #34059, and bug #36828 would all probably require some changes
to the bookmarks file format. However, they're all RFEs, so I'm not sure if they
really go here.

Comment 4

[Jan Varga [:janv]]

sorry, not enough time for these

Comment 5

[Jan Varga [:janv]]

Attachment: Screenshot

Comment 6

[[e-mail]]

Comment on attachment 117763 [details]
Screenshot

I don't see a PT site icon in that screenshot :-P

Comment 7

[Daniel B.]

Also see bug 198420

Comment 8

[Jan Varga [:janv]]

Attachment: UI changes in bookmarks

Comment 9

[Jan Varga [:janv]]

Attachment: everything else

Comment 10

[Pham]

Attachment: error

Is there a reason why those two things are different?

Comment 11

[Pierre Chanial]

Cuc Pham: thanks! I just fixed it.

Comment 12

[Jan Varga [:janv]]

It should be "Address" per original spec in bug 99860.

Comment 13

[Pierre Chanial]

imo, if 'address' is chosen instead of location, then all occurrences of
'location' should be replaced.
but in another bug. Let's keep consistency for now.

Comment 14

[Jan Varga [:janv]]

Attachment: latest diff

Comment 15

[Jan Varga [:janv]]

Changes done on BOOKMARKS_20030310_BRANCH:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=BOOKMARKS_20030310_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=all&mindate=&maxdate=&cvsroot=%2Fcvsroot

Changes done on BOOKMARKS_20030320_BRANCH:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=BOOKMARKS_20030320_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=all&mindate=&maxdate=&cvsroot=%2Fcvsroot

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 118339 [details]
latest diff

I don't think I'm going to  be able to read through every line of this patch,
it's too big. It looks good though.  One comment:

- in nsTreeBodyFrame.cpp:

+      if (primaryX1 > rowRect.x) {
+      }

Jan tells me that he was planning to add code there to deal with the mac native
theme code drawing a vertical line instead of a horizontal separator.  Could
you replace the useless if check with a comment about the problem?

sr=bryner with that change.

Comment 17

[jag (Peter Annema)]

Comment on attachment 118339 [details]
latest diff

r=jag. I went over this with Jan last Friday and Saturday.

Comment 18

[Greg K.]

Is this really PC-only, or is it Hardware=All?

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It looks like the branch landing broke bookmarks in Phoenix.

Comment 20

[Jan Varga [:janv]]

The branch has landed on the trunk.

Comment 21

[Hunter Peress]

This has existed since pre 1.0, and 0 attention has been given:

http://bugzilla.mozilla.org/show_bug.cgi?id=110313

Comment 22

[David G King]

I'm not sure about the zero attention, however, maybe Bug #110313 should be part
of this Meta tracking bug?

Comment 23

[Jan Varga [:janv]]

I filed this bug to track bugs/features we plan to fix on the branch.
The branch landed on the trunk, so other bugs shouldn't be added here.
Leaving this open, because some of the bugs are not verified yet.

Comment 24

[Prognathous]

This bug should probably also depend on:

Bug 59173 (Need better/advanced bookmark/history search)
Bug 93058 ("Type Ahead Find"-like bookmark keyword search)

Prog.

Comment 25

[[e-mail]]

Comment on attachment 118339 [details]
latest diff

+	 var iconNode = document.getAnonymousElementByAttribute(target,
"class", "toolbarbutton-icon");
Did it not occur to you that this can return null?

+    try {
+      return node.QueryInterface(kRDFRSCIID).Value;
+    }
+    catch (e) {
+      return node? node.QueryInterface(kRDFLITIID).Value : "";
+    }	  
instanceof has been around for at least two years...

Comment 26

[[e-mail]]

Were these lines in bookmarks.js added as part of this landing?

QueryInterface : function (aUID) {return this},

This is just so lame it's unbelievable... it royally confuses XPConnect which
subsequently tries to QI your object to all sorts of unsuitable interfaces
including unscriptable ones!

Comment 27

[Pierre Chanial]

Neil: yes, that's part of this landing, and that's also what kin (who wrote the
transaction manager) suggested in the js implementation he sent me via email.
But feel free to patch it.

Comment 28

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614