The link color is picked up directly from the theme, so reassigning to Adwaita.

Created attachment 296057 [details] [review]
Change the default hyperlink color

Use a lighter blue as default color for hyperlinks, readable with dark
background.

There's really a hardcoded 'blue' assignment in the code. The patch above change just that. Now looking at the code this seems to be a fallback default color, so either both the theme and hardcoded value are 'blue', or the code doesn't really get the color from the theme.

Adwaita doesn't use #729fcf as a link color, but #2a76c6 and #4a90d9 for the dark variant, both provides enough contrast to me, so this doesn't look like an Adwaita bug, we don't export a named color for links though, is that the problem?

Looks like it's the problem, indeed.

Here is how polari try to get the color :

        let context = this.widget.get_style_context();
        […]
        let [found, linkColor] = context.lookup_color("link_color");
        if (!found) {
            linkColor = new Gdk.RGBA();
            linkColor.parse('blue');
        }

(I don't know if code pasting is appropriate in bugzilla, tell me if that's not the case)

Also, where should one look at to know which color will integrate well in the gnome environment?

Created attachment 296080 [details] [review]
chatView: Use state flags instead of named color for links

The LINK state flag has replaced the -GtkWidget-link-color property
since 3.12, and the named color was removed from the theme a while
ago.


(In reply to comment #4)
> we don't export a named color for links though, is that the problem?

Yes and no - the theme used to have a named color for links and polari is still using it (or rather: trying to). However there's a new way to get the link color from the style that does not involve named colors, so we just need to start using that and we're good.

Review of attachment 296057 [details] [review]:

The issue is that the color is not picked up from the theme. The fallback color that is used when the color is not found should match the light theme variant that Polari is using.

Review of attachment 296057 [details] [review]:

Works fine for me. I tested with both the default and the dark Adwaita theme. In both case, all looked readable to me.

Comment on attachment 296057 [details] [review]
Change the default hyperlink color

(In reply to comment #9)
> I tested with both the default and the dark Adwaita theme.
> In both case, all looked readable to me.

Just to clarify: I not did mark the patch as rejected because I was unsure how it looked with either variant, it is simply the wrong approach. The color should be picked up from the theme. The existing code does that, and also has a this-should-never-happen fallback path which uses a fixed color when the look up failed. However due to changes in GTK+, we now *always* hit the fallback path - that's the bug that needs to be fixed, not looking for a fallback color that works better with a particular theme (in hindsight, 'blue' was not the best choice and I should have picked something like 'pink' instead - the actual bug would have been apparent for a long time and probably be fixed by now).

That was already clear for me, I didn't expect the patch to be validated as is. I sent a patch as a mean to learn how to use the infrastructure and also I didn't saw your message before I sent it. 

That's your patch that I tested with both theme. Did I reviewed the wrong patch?

(In reply to comment #11)
> That's your patch that I tested with both theme. Did I reviewed the wrong
> patch?

Eeeeks, I'm sorry for the rant then - you did "un-reject" your own patch and set its status to "reviewed", so I was assuming you were making an argument for going with that approach anyway. My apologies, and thanks for testing!

Attachment 296080 [details] pushed as 5ee2fa6 - chatView: Use state flags instead of named color for links