Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

Lightning 4.4
Dependency tree / graph

Details

Attachments

(1 attachment, 3 obsolete attachments)

The Lightning toolbarbutton-icons aren't shown in SM because they are now SVG's and SM doesn't like TB define the icon dimensions. This happens on all platforms except XP which is still using the old icons.
Posted patch SM-toolbarbutton-icon.patch (obsolete) — Splinter Review
Philip, please can you test this with SM if this patch works?

I'm not 100% happy with this patch as we define the dimensions now on TB two times. But I don't see how we could fix this for SM only.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8669910 - Flags: feedback?(philip.chee)
Comment on attachment 8669910 [details] [diff] [review]
SM-toolbarbutton-icon.patch

This works except in the customize palette window. You probably need to style that too.

style chrome://global/content/customizeToolbar.xul chrome://calendar/ chrome://...
Attachment #8669910 - Flags: feedback?(philip.chee) → feedback+
Component: General → Lightning: SeaMonkey Integration
Posted patch SM-toolbarbutton-icon.patch (obsolete) — Splinter Review
Is this better for the customize panel?
Attachment #8669910 - Attachment is obsolete: true
Attachment #8672367 - Flags: feedback?(philip.chee)
Comment on attachment 8672367 [details] [diff] [review]
SM-toolbarbutton-icon.patch

> +++ b/calendar/lightning/themes/common/lightning.css
> .....
> +@media not all and (-moz-os-version: windows-xp) {
> +  .calbar-toolbarbutton-1 .toolbarbutton-icon,
> +  #task-actions-toolbar > .toolbarbutton-1 .toolbarbutton-icon {
Please add:
  .msgHeaderView-button   .toolbarbutton-icon,

> Is this better for the customize panel?
Yes thanks. f+ with the addition above.
Attachment #8672367 - Flags: feedback?(philip.chee) → feedback+
Posted patch SM-toolbarbutton-icon.patch (obsolete) — Splinter Review
This should set all the sizes without interfering TB's styles.
Attachment #8672367 - Attachment is obsolete: true
Attachment #8673816 - Flags: review?(philipp)
Richard, for which version is this report - only 4.6?
Comment on attachment 8673816 [details] [diff] [review]
SM-toolbarbutton-icon.patch

Adding Aurora as I think where will be no new beta build before the next uplift. If I'm wrong then please approve also beta.
Attachment #8673816 - Flags: approval-calendar-aurora?(philipp)
Comment on attachment 8673816 [details] [diff] [review]
SM-toolbarbutton-icon.patch

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

We decided to do one more beta mid cycle from now on, to test changes that may be backported to esr. r/a=philipp
Attachment #8673816 - Flags: review?(philipp)
Attachment #8673816 - Flags: review+
Attachment #8673816 - Flags: approval-calendar-beta+
Attachment #8673816 - Flags: approval-calendar-aurora?(philipp)
Attachment #8673816 - Flags: approval-calendar-aurora+
Keywords: checkin-needed
Version: unspecified → Lightning 4.4
> +  #task-actions-toolbar > .msgHeaderView-button .toolbarbutton-icon {
The task actions toolbar all ready works without this new rule. I was referring to the Customize window for which you need something like:

  toolbarpaletteitem .msgHeaderView-button .toolbarbutton-icon,

(For some reason [.toolbarpaletteitem-box .msgHeaderView-button .toolbarbutton-icon] doesn't work)
This should fix the sizes in customize window.
Attachment #8673816 - Attachment is obsolete: true
Attachment #8675320 - Flags: review?(philipp)
Attachment #8675320 - Flags: approval-calendar-beta?(philipp)
Attachment #8675320 - Flags: approval-calendar-aurora?(philipp)
Keywords: checkin-needed
Attachment #8675320 - Flags: review?(philipp)
Attachment #8675320 - Flags: review+
Attachment #8675320 - Flags: approval-calendar-beta?(philipp)
Attachment #8675320 - Flags: approval-calendar-beta+
Attachment #8675320 - Flags: approval-calendar-aurora?(philipp)
Attachment #8675320 - Flags: approval-calendar-aurora+
Keywords: checkin-needed
Comment on attachment 8675320 [details] [diff] [review]
SM-toolbarbutton-icon.patch

Can we please get this landed now that SeaMonkey 2.39 has been released already and depends on a working beta? Thanks.
Attachment #8675320 - Flags: approval-calendar-release?(philipp)
Lightning 4.5 is currently in comm-beta branch. Unfortunately the current version 4.7 is not yet available for selection in Bugzilla.
Target Milestone: 4.4 → 4.5
And keep in mind that Lightning release channel and repositories contains Lightning 4.0. 
But if I understand your request correctly you want the fix ported back only to Lightning 4.4.
Yes, the initial SVG patch (bug 1153615) landed in 4.4.
Blocks: 1153615
Just realized the missing icon after updating to SM 2.39.

Sorry for the noob user question: How can I apply the patch (Win7)?
You need to rename the .xpi to .zip to unpack or work directly in it and then to add to the correct files (names are in the patch) the parts which have a + on line start (and then remove the + in the files). After saving the files you need to repack the previously unpacked content as zip and rename it again to .xpi. If all is done correctly you can install this xpi and it should work.

But the icons on main toolbar are still not visible and this needs a new patch in a new bug.
Flags: needinfo?(philipp)
Keywords: checkin-needed
Comment on attachment 8675320 [details] [diff] [review]
SM-toolbarbutton-icon.patch

I should probably get that flag renamed. For calendar, we use approval-calendar-release for the last ESR version. We don't do any releases from comm-release, so I don't think this needs approval. If you want this in 4.0.6, please re-request.

All milestones should be there now, please ping me if I missed one.
Flags: needinfo?(philipp)
Attachment #8675320 - Flags: approval-calendar-release?(philipp) → approval-calendar-release-
Resolving as fixed as the fix is in Lightning 4.5 and newer.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [leave open for calendar-release]
Per comment #18, please land this on comm-release (despite the not-quite-matching approval flag) as early as possible for SeaMonkey to pick this up with the 2.40 release coming up soon. Thanks!
Keywords: checkin-needed
Whiteboard: [c-n: comm-release, see comments #18, #20]
(In reply to Martin Schröder [:mschroeder] from comment #19)
> Resolving as fixed as the fix is in Lightning 4.5 and newer.

Err, guess I've been just sleeping now. As long as there is a Lightning 4.5 beta (don't see any so far on AMO) at the time of release, this should work out of the box with SM 2.40, right?

So, what needs to be done to get a 4.5b1 build? After the last merge this patch is now included in comm-release, thus no further action should be necessary on that end.
Flags: needinfo?(philipp)
Keywords: checkin-needed
Whiteboard: [c-n: comm-release, see comments #18, #20]
I've uploaded 4.5b1 now, sorry for the delay. Note again, comm-release is not used by Lightning nor Thunderbird.
Lightning betas are not built from Seamonkey's comm-release, but from Thunderbird's comm-beta.
Flags: needinfo?(philipp)
https://addons.mozilla.org/en-US/firefox/addon/lightning/versions/?page=1#version-4.5b1
Great, thanks! Sorry for my confusion, makes sense.
You need to log in before you can comment on or make changes to this bug.