Closed Bug 1429862 Opened 2 years ago Closed 2 years ago

SM: Restore the toolbar bindings/styles after their removal in bug 1428938

Categories

(SeaMonkey :: General, enhancement)

SeaMonkey 2.57 Branch
enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.57

People

(Reporter: Paenglab, Assigned: stefanh)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 6 obsolete files)

Bug 1428938 removes the legacy toolbar customization code.
Assignee: nobody → stefanh
Attached patch Part 2 (WIP): Use the c-c files (obsolete) — Splinter Review
frg, for some reason I don't get any nav-bar and right now I fail to see why (too tired...). Be warned if you try - the attached patches are on top of the patches in bug 1429232 and bug 1427864.
Flags: needinfo?(frgrahl)
Never mind, I figured it out - I need to investigate the SM-specific bindings that are set in communicator.css since adding "!important" there makes it work...
Flags: needinfo?(frgrahl)
Attached patch wallpaper (obsolete) — Splinter Review
Just putting a wallpaper fix here if someone wants to have a more working SM.
Attachment #8942753 - Attachment description: Part 2: Use the c-c files → Part 2 (WIP): Use the c-c files
Attached patch Part 2: Use the c-c files (obsolete) — Splinter Review
I figured it out - I had forgotten one rule in the themes communicator.css files where the path to the binding wasn't changed (using "!important" in the content communicator.css file made that rule never apply).

This assumes part 1. Patches here are also applied on top of the patches in bug 1429232 and bug 1427864 (it could work without them, but I haven't checked).
Attachment #8942753 - Attachment is obsolete: true
Attachment #8942757 - Attachment is obsolete: true
Attachment #8942965 - Flags: review?(iann_bugzilla)
Comment on attachment 8942752 [details] [diff] [review]
Part 1: Add suite-specific paths in toolbar.xml

Richard, mind take a look at this (need some suite-specific paths)?
Attachment #8942752 - Flags: review?(richard.marti)
Status: NEW → ASSIGNED
Comment on attachment 8942752 [details] [diff] [review]
Part 1: Add suite-specific paths in toolbar.xml

Patch needs a rebase on mail/base/jar.mn. The other things look good.
Attachment #8942752 - Flags: review?(richard.marti) → review+
Comment on attachment 8942965 [details] [diff] [review]
Part 2: Use the c-c files

Minor error in suite/themes/classic/communicator/customizeToolbar.css. Misses a closing bracket:

> +  skin/classic/communicator/customizeToolbar.css                        (communicator/customizeToolbar.css
Thanks frg, here's an updated version.
Attachment #8942965 - Attachment is obsolete: true
Attachment #8942965 - Flags: review?(iann_bugzilla)
Attachment #8944208 - Flags: review?(iann_bugzilla)
Comment on attachment 8942752 [details] [diff] [review]
Part 1: Add suite-specific paths in toolbar.xml

Note to self:
-<!ENTITY % customizeToolbarDTD SYSTEM "chrome://messenger/locale/customizeToolbar.dtd">
+<!ENTITY % customizeToolbarDTD SYSTEM
+
+#ifdef MOZ_SUITE

I should remove the empty line.
Attached patch Part 2 updated (obsolete) — Splinter Review
Attachment #8944208 - Attachment is obsolete: true
Attachment #8944208 - Flags: review?(iann_bugzilla)
Attachment #8944560 - Flags: review?(iann_bugzilla)
Comment on attachment 8944560 [details] [diff] [review]
Part 2 updated

Stealing review. Without the fix SeaMonkey is unusable and I want my toolbars back :)

I assume customizeToolbar.dtd and customizeToolbar.properties came unmodified from mozilla/toolkit/chrome/global. Could you put a note im the new ones to make life for l10n localizers easier.

r+ with that fixed. If IanN finds something else I am happy to fix it myself in a followup bug.
Attachment #8944560 - Flags: review?(iann_bugzilla) → review+
Attachment #8942752 - Attachment is obsolete: true
Attachment #8944560 - Attachment is obsolete: true
Jorg, part 1 modifies common/bindings/toolbar.xml, common/src/customizeToolbar.xul and mail/base/jar.mn.
Keywords: checkin-needed
Version: unspecified → SeaMonkey 2.57 Branch
cc:ing Jorg since I don't know how he watches checkin-needed (comment #16)
Thanks for the CC. I look at checkin-needed multiple times daily, but I my query doesn't see SeaMonkey bugs. It's best to CC, even safer, NI me. I'll land it now.
https://hg.mozilla.org/comm-central/rev/36d28404cd10789371e1edaa755c20d7ad3ef924
SM: Restore the toolbar bindings/styles after their removal in bug 1428938, part 1: Add suite-specific paths in toolbar.xml. r=Paenglab
https://hg.mozilla.org/comm-central/rev/560a1cdd937009d905d0f7e1345bba8a21a90e61
SM: Restore the toolbar bindings/styles after their removal in bug 1428938, part 2: Use the c-c files. r=frg
Pulsebot has taken the weekend off? Also, I can't see target 2.57 :-(
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Future
Thanks Jorg. Yeah, I guess we need to file a bz bug for the targets.
Depends on: 1433717
No longer blocks: 1437595
Depends on: 1437595
Target Milestone: Future → Seamonkey2.57
You need to log in before you can comment on or make changes to this bug.