Closed
Bug 1331208
Opened 8 years ago
Closed 7 years ago
menulist-button is not visible in Classic (Default) theme under gtk3
Categories
(SeaMonkey :: Themes, defect)
Tracking
(seamonkey2.49esr fixed, seamonkey2.56 fixed, seamonkey2.53+ fixed)
RESOLVED
FIXED
seamonkey2.56
People
(Reporter: iannbugzilla, Assigned: frg)
References
(Blocks 1 open bug)
Details
Attachments
(7 files, 5 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
iannbugzilla
:
review+
iannbugzilla
:
approval-comm-esr52+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
stefanh
:
review+
|
Details | Diff | Splinter Review |
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.
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•8 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•8 years ago
|
||
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•8 years ago
|
Attachment #8826866 -
Flags: feedback?(frgrahl) → feedback-
Comment 6•8 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-
(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)
Comment 9•8 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•8 years ago
|
||
It still should work in gtk2 builds without introducing a regression there, obviously.
Assignee | ||
Comment 11•8 years ago
|
||
TB Bug 1332867 might be the same problem.
Updated•8 years ago
|
Comment 12•8 years ago
|
||
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•8 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.
Blocks: SM2.49-GTK3
Comment 14•7 years ago
|
||
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•7 years ago
|
Assignee: wgianopoulos → nobody
Status: ASSIGNED → NEW
Assignee | ||
Comment 15•7 years ago
|
||
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 17•7 years ago
|
||
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•7 years ago
|
||
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 19•7 years ago
|
||
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•7 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.
Comment 21•7 years ago
|
||
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;"?
Comment 22•7 years ago
|
||
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•7 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 :)
Comment 24•7 years ago
|
||
(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.
Reporter | ||
Comment 26•7 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•7 years ago
|
||
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•7 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•7 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 30•7 years ago
|
||
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•7 years 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?
Comment 32•7 years ago
|
||
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•7 years 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•7 years 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•7 years 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+
Comment 36•7 years ago
|
||
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•7 years 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•7 years ago
|
Keywords: leave-open
Comment 38•7 years ago
|
||
(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 39•7 years ago
|
||
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•7 years 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 | ||
Comment 41•7 years ago
|
||
Comment on attachment 8937350 [details] [diff] [review]
1331208-gtkdropmarker-part1.patch
Part 1 fixed for 2.49.2:
https://hg.mozilla.org/releases/comm-esr52/rev/76f90fc74cc948ced1275590757481142a251776
Assignee | ||
Updated•7 years ago
|
status-seamonkey2.49esr:
--- → fixed
status-seamonkey2.56:
--- → affected
Assignee | ||
Updated•7 years ago
|
No longer blocks: SeaMonkey2.49.2ESR
Reporter | ||
Comment 42•7 years ago
|
||
Comment on attachment 8941207 [details] [diff] [review]
1331208-gtkdropmarker-part2.patch
f=me
Attachment #8941207 -
Flags: feedback?(iann_bugzilla) → feedback+
Assignee | ||
Comment 43•7 years 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•7 years ago
|
Keywords: leave-open
Updated•7 years ago
|
Attachment #8942178 -
Flags: review?(stefanh) → review+
Comment 44•7 years 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
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•7 years ago
|
status-seamonkey2.53:
--- → affected
Target Milestone: --- → Seamonkey2.56
Assignee | ||
Comment 46•4 years ago
|
||
Target 2.53.1
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/455a37659eb60182471e325fb4678a50d99fae4d
tracking-seamonkey2.53:
--- → +
You need to log in
before you can comment on or make changes to this bug.
Description
•