Closed Bug 1331208 Opened 7 years ago Closed 6 years ago

menulist-button is not visible in Classic (Default) theme under gtk3

Categories

(SeaMonkey :: Themes, defect)

All
Linux
defect
Not set
normal

Tracking

(seamonkey2.49esr fixed, seamonkey2.56 fixed, seamonkey2.53+ fixed)

RESOLVED FIXED
seamonkey2.56
Tracking Status
seamonkey2.49esr --- fixed
seamonkey2.56 --- fixed
seamonkey2.53 + fixed

People

(Reporter: iannbugzilla, Assigned: frg)

References

(Blocks 1 open bug)

Details

Attachments

(7 files, 5 obsolete files)

The menulist-button is used as the drop down marker for things like:
1/ URL bar history drop down
2/ Multiple identities drop down in compose from field

Both work fine under modern theme or when using 2.46 with gtk2

Steps to Reproduce
1/ Start trunk build of SeaMonkey (2.50)
2/ Try and find drop down marker for URL bar history

Expected Results
1/ Drop down marker is visible and can be clicked on to produce a drop down.

Actual Results
1/ Drop down marker cannot be seen (it is there but only 2px wide)

Workarounds
a) Switch to modern theme
b) Go to the right border of text box and click, you might hit the near invisible marker.
Attached patch WIP patch (obsolete) — Splinter Review
This fixes the symptoms in the two places that I have found the issue. There is probably a better place to fix the symptoms and the cause should really be identified (perhaps a change in Core).
It may also break other platforms.
Attachment #8826866 - Flags: feedback?(stefanh)
Attachment #8826866 - Flags: feedback?(philip.chee)
Attachment #8826866 - Flags: feedback?(frgrahl)
Comment on attachment 8826866 [details] [diff] [review]
WIP patch

These changes doesn't affect mac since we have suite/themes/classic/mac/messenger/messengercompose/messengercompose.css and 
suite/themes/classic/mac/navigator/navigator.css for this.
Attachment #8826866 - Flags: feedback?(stefanh) → feedback+
Attached image Clipboard.png
The patch makes the location bar too high on windows. It now also has a white border around when using https (not in the screenshot.
Attachment #8826866 - Flags: feedback?(frgrahl) → feedback-
Comment on attachment 8826866 [details] [diff] [review]
WIP patch

> +#msgIdentity[editable="true"] > .menulist-dropmarker {
> +  border-width: 4px 6px;
> +  width: 32px;
> +  height: 30px;
Did you poke at this with the DOM inspector?
Is there a regression range?
Does some combination of -moz-appearance: none; AND/OR display: -moz-box; work?
Please don't regress gtk2, osx, and windows thanks muchly.
Also did you try with a different GNOME/GTK3 theme?

https://dxr.mozilla.org/comm-central/search?q=%25ifdef+MOZ_WIDGET_GTK&redirect=false

Absent separate files for gtk in our classic theme you could use %ifdef MOZ_WIDGET_GTK to limit your changes.
Attachment #8826866 - Flags: feedback?(philip.chee) → feedback-
(In reply to Philip Chee from comment #6)
> Comment on attachment 8826866 [details] [diff] [review]
> WIP patch
> 
> > +#msgIdentity[editable="true"] > .menulist-dropmarker {
> > +  border-width: 4px 6px;
> > +  width: 32px;
> > +  height: 30px;
> Did you poke at this with the DOM inspector?
Yes, I did poke with DOM inspector.

> Is there a regression range?
Yes, last working build:
http://ftp.mozilla.org/pub/seamonkey/nightly/2015/07/2015-07-23-00-30-01-comm-central-trunk/seamonkey-2.39a1.en-US.linux-x86_64.tar.bz2
First non-working build:
http://ftp.mozilla.org/pub/seamonkey/nightly/2015/08/2015-08-01-00-30-01-comm-central-trunk/seamonkey-2.39a1.en-US.linux-x86_64.tar.bz2
Unfortunately no nightly builds of linux between those dates to narrow any further. Gives a regression range on m-c of:
https://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2015-07-23&enddate=2015-08-01
> Does some combination of -moz-appearance: none; AND/OR display: -moz-box; work?
display: -moz-box; is what it has already.
-moz-appearance: menulist-button; is what it has to start with, setting to none gives the drop marker a width of zero.
> Please don't regress gtk2, osx, and windows thanks muchly.
I wasn't planning too, this is a WIP patch.
> Also did you try with a different GNOME/GTK3 theme?
Yes, tried with a couple of themes, made no difference. Happy to try more if someone can point me at some decent ones.
Well all that regression range told me is that nightlies switched from GTK2 to GTK3 as the default (see bug 1186229)
Can you try enclosing your changes with %ifdef MOZ_WIDGET_GTK ... %endif?
Do we have to worry about distros still on GTK2?
It still should work in gtk2 builds without introducing a regression there, obviously.
TB Bug 1332867 might be the same problem.
See Also: → 1332867
This is a WORKSFORME with my Sm 2.49 64 bit GTK3 build (20170130041559) on Kubuntu 14.04.

I'd guess this is one of the many GTK3 problems that are dependent on the Theme chosen for GTK3 (mine is "Clearlooks-Phenix"). Nonetheless, we need to override such a theme behavior...
(In reply to Adrian Kalla [:adriank] from comment #12)
> This is a WORKSFORME with my Sm 2.49 64 bit GTK3 build (20170130041559) on
> Kubuntu 14.04.
> 
> I'd guess this is one of the many GTK3 problems that are dependent on the
> Theme chosen for GTK3 (mine is "Clearlooks-Phenix"). Nonetheless, we need to
> override such a theme behavior...

This also depends on GTK3 version. In the TB bug we only see it with GTK above 3.18, not older.
Blocks: SM2.49-GTK3
I am taking this bug.  I am going to engage the people who worked on Firefox issues on gtk2 to gtk3 conversion.  We can probably workaround this, but if the issue is really in the GTK widget code it should really be fixed there before it comes back to bite someone again.
Assignee: nobody → wgianopoulos
Status: NEW → ASSIGNED
Assignee: wgianopoulos → nobody
Status: ASSIGNED → NEW
Attached image Capture.PNG
This now has crept into CentOS too. The drop-marker is still there but 1 px wide. I think I saw a patch in TB CSS.
Lets see if we can fix it for 2.49.2
Attached patch 1331208-gtkdropmarker.patch (obsolete) — Splinter Review
The patch from TB also works fine for SeaMonkey. With 2em the dropmarker is a little wider under Windows but does not look bad. Height is fine
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Attachment #8931964 - Flags: review?(stefanh)
Attachment #8931964 - Flags: review?(iann_bugzilla)
Attached patch 1331208-gtkdropmarker.patch (obsolete) — Splinter Review
Updated for all mail, news and calendar dialogs. Tested with 2.49.2 under Linux and 2.53 under Windows. I didn't see any others outside mail but they may be some. Should we put it in communicator.css?
Attachment #8931964 - Attachment is obsolete: true
Attachment #8931964 - Flags: review?(stefanh)
Attachment #8931964 - Flags: review?(iann_bugzilla)
Attachment #8931972 - Flags: review?(stefanh)
Attachment #8931972 - Flags: review?(iann_bugzilla)
Comment on attachment 8931972 [details] [diff] [review]
1331208-gtkdropmarker.patch

 .autocomplete-history-dropmarker {
+  min-width: 2em; /* Fix to show the dropmarker under newer GTK3 versions */
   border-right-width: 1px;

Hmm, if you don't need the ifdef MOZ_WIDGET_GTK here, any reason to have it in messenger.css?
> Hmm, if you don't need the ifdef MOZ_WIDGET_GTK here, any reason to have it in messenger.css?

This also affects OSX in messenger.css and the marker also gets a little wider in Windows. It is fine for the urlbar and the marker in IanNs original patch but I just wanted to play it safe here because it needs to go to the ESR version too.
So you get this from the linux menulist.css in toolkit:

menulist[editable="true"] > .menulist-dropmarker {
  display: -moz-box;
  -moz-appearance: menulist-button;
}

Does the dropmarker differ from the autocomplete one? What happens if you override that with "-moz-appearance: none;"?
And does the linux dropmarker looks like the windows one? The windows dropmarker isn't drawn by widget code, it's an icon (https://dxr.mozilla.org/comm-central/rev/3e14872b31a7b1b207605d09b78fbaaf21f1bba7/mozilla/toolkit/themes/windows/global/dropmarker.css#12). Maybe that one could be used for linux (just a thought)?
> Does the dropmarker differ from the autocomplete one? What happens if you override that with "-moz-appearance: none;"?

yes the autocomplete one seems to have lost all styling in the latest updates. It is even different in 2.49.1 now. Arrow is there but background color and highlighting no longer works. This probably needs to be fixed in a future update.

Still not the most skilful css guru out there so appreciate the help :)
(In reply to Frank-Rainer Grahl (:frg) from comment #23)
> > Does the dropmarker differ from the autocomplete one? What happens if you override that with "-moz-appearance: none;"?
> 
> yes the autocomplete one seems to have lost all styling in the latest
> updates.

If it's about linux, it might be https://hg.mozilla.org/mozilla-central/rev/b80b14cd26f3#l10.12 and further down.
Comment on attachment 8931972 [details] [diff] [review]
1331208-gtkdropmarker.patch

r/a=me for esr
Attachment #8931972 - Flags: review?(iann_bugzilla)
Attachment #8931972 - Flags: review+
Attachment #8931972 - Flags: approval-comm-esr52+
Attached image dropmarker_cc.png
On c-c there's an extra gap between the drop marker and the end of the address bar
Better than not working at all, so happy for it to land to fix the issue, but I think there is an extra gap at the start too.
> On c-c there's an extra gap between the drop marker and the end of the address bar

I see it and need to check it without the patch but it seems to me we pick up some other unwanted styling here not part of this bug. Probably Photon and only location bar related. The dropmarkers in mail and calendar look fine in c-c.
Attached patch 1331208-gtkdropmarker-V2.patch (obsolete) — Splinter Review
Bug 464450 and Bug 1407613 changed the appearance for c-c.

Comments in the bugs indicate that these where rather hacks. Should we put them in our code or try to fix them in a followup bug. I would be for fixing it but might not happen in this life becuase of other prio 1s unfortunately....
Attachment #8936297 - Flags: feedback?(stefanh)
Attachment #8936297 - Flags: feedback?(iann_bugzilla)
Comment on attachment 8936297 [details] [diff] [review]
1331208-gtkdropmarker-V2.patch

--- a/suite/themes/classic/communicator/communicator.css
+++ b/suite/themes/classic/communicator/communicator.css
@@ -15,16 +15,36 @@
 @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
 
 /* ::::: toolbar-primary ::::: */
 
 .toolbar-primary {
   -moz-binding: url("chrome://communicator/content/bindings/toolbar.xml#grippytoolbar-primary");
 }
 
+/* bug 464450 */
+/* .padded is used by autocomplete widgets that don't have an icon. Gross. -dwh */
+textbox:not(.padded) {
+  cursor: default;
+  padding: 0;
+}

How does this look in Linux (the above style rules are from a toolkit windows file)?
The first fix was good enough for ESR 52 but I think moving it to communicator.css is better. I tested it with 2.49.2 (CentOS) and 2.53 (Windows) and it should take care of any dropmarker now. Didn't see any regressions under Windows and Linux. OSX not affected.

Will see that I put an additonal patch from 1331208-gtkdropmarker-V2.patch for 2.56+ on top and put a picture up the for Stefan.
Attachment #8931972 - Attachment is obsolete: true
Attachment #8931972 - Flags: review?(stefanh)
Attachment #8937350 - Flags: review?(iann_bugzilla)
Attachment #8937350 - Flags: approval-comm-esr52?
For gecko59, I'm wondering if having a themes/classic/linux/communicator/communicator.css file would be an option. The m-c xbl removal will probably mean that we lose Linux-specific style rules for some widgets unless we migrate the rules to a Linux-specific file.
Sounds like a plan. gtk3 seems to need a lot of tinkering anyway a seperate dir, like in toolkit, would probably help.
Comment on attachment 8937350 [details] [diff] [review]
1331208-gtkdropmarker-part1.patch

r/a=me for ESR
for trunk I do like the idea of splitting off into a linux subdirectory where required.
Attachment #8937350 - Flags: review?(iann_bugzilla)
Attachment #8937350 - Flags: review+
Attachment #8937350 - Flags: approval-comm-esr52?
Attachment #8937350 - Flags: approval-comm-esr52+
Comment on attachment 8936297 [details] [diff] [review]
1331208-gtkdropmarker-V2.patch

I know it is perhaps outside the scope of this bug but the URL bar, drop down marker, Go button, the Search field and the Search button are all slightly different heights. It would look better if they matched.

As mentioned elsewhere, for trunk split off a linux version of communicator.css
Attachment #8936297 - Flags: feedback?(iann_bugzilla) → feedback+
FWIW, in https://bug1424602.bmoattachments.org/attachment.cgi?id=8938901 I'm adding a suite/themes/classic/linux/communicator/preferences.css file
Rebased part 2 which builds on part 1 and duplicates communicator.css. Works fine in Windows but I need to retest Linux and probably adjust some things. 

jar.mn might become quite messy overtime. Should we restructure the theme in a separate bug and add a shared dir plus separate mn files like in mozilla/browser?
Attachment #8826866 - Attachment is obsolete: true
Attachment #8936297 - Attachment is obsolete: true
Attachment #8936297 - Flags: feedback?(stefanh)
Attachment #8941207 - Flags: feedback?(stefanh)
Attachment #8941207 - Flags: feedback?(iann_bugzilla)
Keywords: leave-open
(In reply to Frank-Rainer Grahl (:frg) from comment #37) 
> jar.mn might become quite messy overtime. Should we restructure the theme in
> a separate bug and add a shared dir plus separate mn files like in
> mozilla/browser?

Sounds like a plan. But maybe wait a bit - I'm pretty sure there are more urgent things to fix :-).

Re the patch: I don't have any means of testing it, but I think it makes more sense to point to bugs in these files when the code is a workaround for a problem (like a layout issue), so I'd put the bug numbers in the commit message instead (you'll then see them in the blame).
Comment on attachment 8941207 [details] [diff] [review]
1331208-gtkdropmarker-part2.patch

But the patch looks OK to me (modulo the bug numbers).
Attachment #8941207 - Flags: feedback?(stefanh) → feedback+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/694dba22656c
Fix to show the menulist- and history-dropmarker under newer GTK3 versions in SeaMonkey. r=IanN
No longer blocks: SeaMonkey2.49.2ESR
Comment on attachment 8941207 [details] [diff] [review]
1331208-gtkdropmarker-part2.patch

f=me
Attachment #8941207 - Flags: feedback?(iann_bugzilla) → feedback+
Removed the bug numbers from the source and retested on Linux and Windows. Works for now:)
Attachment #8941207 - Attachment is obsolete: true
Attachment #8942178 - Flags: review?(stefanh)
Keywords: leave-open
Attachment #8942178 - Flags: review?(stefanh) → review+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/29104fe7818d
Part 2. Re-add padded removed in Bug 464450 and the dropmarker removed in Bug 1407613. r=stefanh
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Seamonkey2.56
You need to log in before you can comment on or make changes to this bug.