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

RESOLVED FIXED in seamonkey2.56

Status

defect
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: → 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: 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

Updated

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

Comment 15

2 years 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

2 years ago
Lets see if we can fix it for 2.49.2
Assignee

Comment 17

2 years 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

2 years 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

2 years 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

2 years 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

2 years ago
Duplicate of this bug: 1392100
Reporter

Comment 26

2 years 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

2 years 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

2 years 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

2 years 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
No longer blocks: SeaMonkey2.49.2ESR
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
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.