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

RESOLVED FIXED in seamonkey2.56

Status

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: iann_bugzilla, Assigned: frg)

Tracking

(Blocks: 2 bugs)

Trunk
seamonkey2.56
All
Linux
Dependency tree / graph

SeaMonkey Tracking Flags

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

Details

Attachments

(7 attachments, 5 obsolete attachments)

(Reporter)

Description

2 years ago
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.
(Reporter)

Comment 1

2 years ago
(Reporter)

Comment 2

2 years ago
(Reporter)

Comment 3

2 years ago
Posted 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 4

2 years ago
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+
(Assignee)

Comment 5

2 years ago
Posted 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.
(Assignee)

Updated

2 years ago
Attachment #8826866 - Flags: feedback?(frgrahl) → feedback-

Comment 6

2 years ago
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-
(Reporter)

Comment 7

2 years ago
(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.
(Reporter)

Comment 8

2 years ago
Well all that regression range told me is that nightlies switched from GTK2 to GTK3 as the default (see bug 1186229)

Comment 9

2 years ago
Can you try enclosing your changes with %ifdef MOZ_WIDGET_GTK ... %endif?
Do we have to worry about distros still on GTK2?

Comment 10

2 years ago
It still should work in gtk2 builds without introducing a regression there, obviously.
(Assignee)

Comment 11

2 years ago
TB Bug 1332867 might be the same problem.

Updated

2 years ago
See Also: → bug 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...

Comment 13

2 years ago
(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.

Updated

2 years ago
Blocks: 1367257
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
Blocks: 1264079
(Assignee)

Updated

2 years ago
Assignee: wgianopoulos → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 15

a year ago
Posted 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.
(Assignee)

Comment 16

a year ago
Lets see if we can fix it for 2.49.2
Blocks: 1420707
(Assignee)

Comment 17

a year ago
Posted 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)
(Assignee)

Comment 18

a year ago
Posted 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?
(Assignee)

Comment 20

a year ago
> 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)?
(Assignee)

Comment 23

a year ago
> 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.
(Assignee)

Updated

a year ago
Duplicate of this bug: 1392100
(Reporter)

Comment 26

a year ago
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+
(Reporter)

Comment 27

a year ago
Posted 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.
(Assignee)

Comment 28

a year ago
> 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.
(Assignee)

Comment 29

a year ago
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)?
(Assignee)

Comment 31

a year ago
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.
(Assignee)

Comment 33

a year ago
Sounds like a plan. gtk3 seems to need a lot of tinkering anyway a seperate dir, like in toolkit, would probably help.
(Reporter)

Comment 34

a year ago
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+
(Reporter)

Comment 35

a year ago
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
(Assignee)

Comment 37

a year ago
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)
(Assignee)

Updated

a year ago
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+

Comment 40

a year ago
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
(Assignee)

Updated

a year ago
status-seamonkey2.49esr: --- → fixed
status-seamonkey2.56: --- → affected
(Assignee)

Updated

a year ago
No longer blocks: 1420707
(Reporter)

Comment 42

a year ago
Comment on attachment 8941207 [details] [diff] [review]
1331208-gtkdropmarker-part2.patch

f=me
Attachment #8941207 - Flags: feedback?(iann_bugzilla) → feedback+
(Assignee)

Comment 43

a year ago
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)
(Assignee)

Updated

a year ago
Keywords: leave-open

Updated

a year ago
Attachment #8942178 - Flags: review?(stefanh) → review+

Comment 44

a year ago
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
Last Resolved: a year ago
Resolution: --- → FIXED
(Assignee)

Updated

a year ago
status-seamonkey2.53: --- → affected
status-seamonkey2.56: affected → fixed
Target Milestone: --- → Seamonkey2.56
(Assignee)

Updated

a year ago
Duplicate of this bug: 1433720
You need to log in before you can comment on or make changes to this bug.