Closed Bug 1314928 Opened 8 years ago Closed 7 years ago

-moz-nativehyperlinktext is blue rather than orange on Ubuntu 16.10

Categories

(Core :: Widget: Gtk, defect, P2)

Product:
Core
Component:
Widget: Gtk
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox52 --- wontfix
firefox57 --- fixed

People

(Reporter: dao, Assigned: stransky)

References

Details

(Whiteboard: tpi:+)

Attachments

(1 file)

Bug 1314928 - get link text color by GTK_STATE_FLAG_LINK on Gtk3 >= 3.12,
59 bytes, text/x-review-board-request
karlt
: review+
Details 
I just upgraded from Ubuntu 16.04 to 16.10 and -moz-nativehyperlinktext is now blue rather than orange. Links are still orange in other applications, e.g. in GNOME Terminal's and gedit's about windows and help viewers.
Priority: -- → P2
Whiteboard: tpi:+
This currently affects Nightly (54).

I don't know if this is merely cosmetic on Ubuntu, but it's a real usability problem with dark GTK themes in general; I'm using Adwaita Dark on Arch and on one of my monitors, the default -moz-nativehyperlinktext color is very difficult to read against dark backgrounds.
Mass wontfix for bugs affecting firefox 52.
status-firefox52: affected → wontfix
(In reply to Julien Cristau [:jcristau] from comment #2)
> Mass wontfix for bugs affecting firefox 52.

I hope this is just a wontfix for the 52 branch and not a general wontfix? Like I said, I observed the same issue out in 54.
(In reply to ryan.hendrickson from comment #1)
> This currently affects Nightly (54).
> 
> I don't know if this is merely cosmetic on Ubuntu, but it's a real usability
> problem with dark GTK themes in general; I'm using Adwaita Dark on Arch and
> on one of my monitors, the default -moz-nativehyperlinktext color is very
> difficult to read against dark backgrounds.

This is targeted at Bug 1158076 - issues with dark themes.
As noted in bug 1386934, the hyperlink should have a brighter color when displayed on dark background in the awesomebar.

:dao says that this is within the scope of this bug.
Martin, the problem reported in bug 1386934 and in comment 1 has not been fixed by bug 1158076.

I'm on latest nightly, in Gnome with global dark theme and the links are dark blue in the awesomebar.

Does it mean that your comment 4 was incorrect, or didn't complete the fix?
Flags: needinfo?(stransky)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #6)
> Martin, the problem reported in bug 1386934 and in comment 1 has not been
> fixed by bug 1158076.
> 
> I'm on latest nightly, in Gnome with global dark theme and the links are
> dark blue in the awesomebar.
> 
> Does it mean that your comment 4 was incorrect, or didn't complete the fix?

There are two different situations there which has to be targeted:

1) gnome3 global dark theme is used. It means that the current Gtk3 theme has light/dark variants and the dark one is used. That can be easily detected and the switch is implemented at Bug 1158076.

Bug 1158076 allows to use light theme for web content (that's what web designers expect) and global dark theme for Firefox UI. We should adjust awesome bar colors (and probably other colors) in that case because FF UI theme does not expect the dark colors here. That may be handled at Bug 1386934.

2) A theme with dark variant only is used. It means that the global gtk3 "dark theme" switch is off and gnome indicates the light theme is used but the actual theme has dark colors (like Arc on KDE). We may also detect that situation and adjust the FF UI colors accordingly.
Flags: needinfo?(stransky)
(In reply to Martin Stránský from comment #7)
> 2) A theme with dark variant only is used. It means that the global gtk3
> "dark theme" switch is off and gnome indicates the light theme is used but
> the actual theme has dark colors (like Arc on KDE). We may also detect that
> situation and adjust the FF UI colors accordingly.

Maybe we should implement some widget level function which will detect the global dark theme and can be queried by FF UI. That code should check both the gnome3 global dark theme switch and also try to read window background/foreground colors from widget. Karl, what do you think?
Flags: needinfo?(karlt)
BTW I'll also look at the wrong -moz-nativehyperlinktext color - that may fix FF usability on dark themes until we have some better solution.
Maybe (hopefully) this is what you mean by ‘wrong -moz-nativehyperlinktext color’, but I'm confused by all this talk about detecting the Gnome/GTK theme and whether it's dark or not and ‘adjust[ing] FF UI colors accordingly’... why isn't the best solution to just read the current GTK hyperlink color and use that as -moz-nativehyperlinktext, trusting that the GTK theme has made usable choices? Non-native Firefox themes should be setting their own hyperlink colors instead of using -moz-nativehyperlinktext, shouldn't they? And if web content should be presented with consistent styles across OSes, the default presentation for links in content shouldn't be literally -moz-nativehyperlinktext; it should be -moz-hyperlinktext, which can remain a conservative shade of dark blue. I'm puzzled that this doesn't sound like the approach you're defaulting to.
(In reply to ryan.hendrickson from comment #10)
> Maybe (hopefully) this is what you mean by ‘wrong -moz-nativehyperlinktext
> color’, but I'm confused by all this talk about detecting the Gnome/GTK
> theme and whether it's dark or not and ‘adjust[ing] FF UI colors
> accordingly’... why isn't the best solution to just read the current GTK
> hyperlink color and use that as -moz-nativehyperlinktext, trusting that the
> GTK theme has made usable choices?

Right, the talk about about dark themes doesn't have much to do with this bug. This bug is about -moz-nativehyperlinktext getting the default blue rather than what GTK themes specify.
(In reply to Dão Gottwald [::dao] from comment #11)
> (In reply to ryan.hendrickson from comment #10)
> > Maybe (hopefully) this is what you mean by ‘wrong -moz-nativehyperlinktext
> > color’, but I'm confused by all this talk about detecting the Gnome/GTK
> > theme and whether it's dark or not and ‘adjust[ing] FF UI colors
> > accordingly’... why isn't the best solution to just read the current GTK
> > hyperlink color and use that as -moz-nativehyperlinktext, trusting that the
> > GTK theme has made usable choices?
> 
> Right, the talk about about dark themes doesn't have much to do with this
> bug. This bug is about -moz-nativehyperlinktext getting the default blue
> rather than what GTK themes specify.

Okay, sorry for the confusion here. My comment 7 should be more appropriate for the Bug 1158076.
(In reply to Martin Stránský from comment #8)
> Maybe we should implement some widget level function which will detect the
> global dark theme and can be queried by FF UI. That code should check both
> the gnome3 global dark theme switch and also try to read window
> background/foreground colors from widget. Karl, what do you think?

I'm not sure whether this is still relevant in the light of comment 11 et al.

There may be times when it is useful to know whether the theme is generally
dark or light, but it would be helpful to understand the exact need.  Themes
may be generally dark, but have light input boxes, for example.

Chrome should only use native background colors with the associated native
foreground colors and vice versa.

There are times when chrome wishes to use a native background color, but have
more variations in foreground color than provided by the theming API.  I don't
have a good suggestion there short of chrome considering the background color
when selecting the foreground colors.
Flags: needinfo?(karlt)
I can confirm that the patch fixes the papercut I reported in bug 1386934.
Comment on attachment 8899502 [details]
Bug 1314928 - get link text color by GTK_STATE_FLAG_LINK on Gtk3 >= 3.12,

https://reviewboard.mozilla.org/r/170780/#review177648

::: commit-message-c7c96:1
(Diff revision 1)
> +Bug 1314928 - get link text color by GTK_STATE_FLAG_LINK on Gtk3 > 3.12, r?karlt

>= 3.12

::: widget/gtk/nsLookAndFeel.cpp:1466
(Diff revision 1)
> +        style = gtk_widget_get_style_context(linkButton);
> +        gtk_style_context_get_color(style, GTK_STATE_FLAG_LINK, &color);

Please add a comment to indicate that this will miss colors set by a theme only on the label.
It is the GtkLabel that draws with the "color" property.
Attachment #8899502 - Flags: review?(karlt) → review+
Comment on attachment 8899502 [details]
Bug 1314928 - get link text color by GTK_STATE_FLAG_LINK on Gtk3 >= 3.12,

https://reviewboard.mozilla.org/r/170780/#review177648

> Please add a comment to indicate that this will miss colors set by a theme only on the label.
> It is the GtkLabel that draws with the "color" property.

Fixed. The question is if that's even possible to have a theme with broken link color for GtkLinkButton.
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f349caf1814
get link text color by GTK_STATE_FLAG_LINK on Gtk3 >= 3.12, r=karlt
Keywords: checkin-needed
Assignee: nobody → stransky
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #21)
> Backed out for bustage at widget/gtk/nsLookAndFeel.cpp:47: invalid
> conversion from 'int' to 'GtkStateFlags':
> 
> https://hg.mozilla.org/integration/autoland/rev/
> 0a07372e5318401d3a948577732cbc66e86d38fe
> 
> Push with bustage:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=1f349caf18145e38390935780775372c343b340d&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-resultStatus=retry&filter-
> resultStatus=usercancel&filter-resultStatus=runnable
> Build log:
> https://treeherder.mozilla.org/logviewer.html#?job_id=125829645&repo=autoland
> >  /home/worker/workspace/build/src/widget/gtk/nsLookAndFeel.cpp:47:32: error: invalid conversion from 'int' to 'GtkStateFlags' [-fpermissive]

Fixed by the next commit, Thanks.
Flags: needinfo?(stransky)
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30f93fe09f99
get link text color by GTK_STATE_FLAG_LINK on Gtk3 >= 3.12, r=karlt
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/30f93fe09f99
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: