Closed Bug 196756 Opened 17 years ago Closed 10 years ago

Meta bug for major architectural changes in bookmarks code


(SeaMonkey :: Bookmarks & History, defect)

Not set


(Not tracked)



(Reporter: janv, Unassigned)



(Keywords: meta)


(3 files, 2 obsolete files)

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.
OS: Linux → All
Priority: -- → P1
Target Milestone: --- → mozilla1.4beta
How about bug 82721, then, which blocks bug 89001?
Depends on: 194423
QA Contact: kasumi → petersen
Depends on: 166142
Depends on: 140425
Depends on: 140418
Depends on: 124819
Depends on: 123549
I also included some cleanup bugs and minor issues, sorry for the SPAM.
Depends on: 119796
Depends on: 160019
Depends on: 120531
Depends on: 146859
Depends on: 117449
Depends on: 191783
Depends on: 36339
Depends on: 82721
Depends on: 121249
Depends on: 121053
Depends on: 177213
No longer depends on: 118393
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.
sorry, not enough time for these
Depends on: 113574
Depends on: 114962
No longer depends on: 114962
Depends on: 114962
Attached image Screenshot
No longer depends on: 77411
No longer depends on: 191783
Comment on attachment 117763 [details]

I don't see a PT site icon in that screenshot :-P
Also see bug 198420
Attached patch UI changes in bookmarks (obsolete) — Splinter Review
Attached patch everything else (obsolete) — Splinter Review
Attached image error
Is there a reason why those two things are different?
Cuc Pham: thanks! I just fixed it.
It should be "Address" per original spec in bug 99860.
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.
Attachment #118114 - Attachment is obsolete: true
Attachment #118115 - Attachment is obsolete: true
Attached file latest diff
Attachment #118339 - Attachment mime type: application/x-tar → application/x-gzip
Attachment #118339 - Flags: superreview?(bryner)
Attachment #118339 - Flags: review?(jaggernaut)
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.
Attachment #118339 - Flags: superreview?(bryner) → superreview+
Comment on attachment 118339 [details]
latest diff

r=jag. I went over this with Jan last Friday and Saturday.
Attachment #118339 - Flags: review?(jaggernaut) → review+
Is this really PC-only, or is it Hardware=All?
Keywords: meta
It looks like the branch landing broke bookmarks in Phoenix.
The branch has landed on the trunk.
Depends on: 199328
This has existed since pre 1.0, and 0 attention has been given:
I'm not sure about the zero attention, however, maybe Bug #110313 should be part
of this Meta tracking bug?
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.
This bug should probably also depend on:

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

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...
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!
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.
Product: Browser → Seamonkey
Assignee: Jan.Varga → nobody
Priority: P1 → --
QA Contact: chrispetersen → bookmarks
Target Milestone: mozilla1.4beta → ---
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
Closed: 10 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.