Community
Participate
Working Groups
Created attachment 268613 [details] screenshot of dark blue text in popup When hovering over any method, the hyper-links in the javadoc pop-up are a dark blue color over a black background making it difficult to read. This does not follow the color set in the preferences.
I'll investigate. I think there's an issue in how HTMLPrinter produces the href tags.
*** Bug 517709 has been marked as a duplicate of this bug. ***
HTMLPrinter#convertTopLevelFont uses e.g. the regular expression: (html\s*\{.*(?:\\s|;)font-size:\s*)\d+pt(\;?.*\}) to e.g. replace the "9pt" in JDT's JavadocViewStyleSheet.css: html { font-family: sans-serif; font-size: 9pt; font-style: normal; font-weight: normal; } with the current configured font height. We could do something similar with the "color" attribute of links. Regarding links the JDT's JavadocViewStyleSheet.css contains: a:link { color: #0000FF; } a:hover { color: #000080; } a:visited { text-decoration: underline; } a.header:link { text-decoration: none; color: InfoText } a.header:visited { text-decoration: none; color: InfoText } a.header:hover { text-decoration: underline; color: #000080; } so there are 3 possible colors for links: "link" = unvisited link "hover" = when the user mouses over it "visited"= visited link see https://www.w3schools.com/css/css_link.asp The "Colors and Font" preference page provides only two colors: "Basic -> Active hyperlink text color" and "Basic -> Hyperlink text color". How should we handle this case? I fiddled around a bit with https://regex101.com/ and crafted the following regex: (a[.[:alnum:]]*:link\s*\{.*(?:\s|;)color:\s*)[^\s\}^;]+(\;?.*\}) that fit for the a:link { color: #0000FF; } and the a.header:link { text-decoration: none; color: InfoText } line in JDT's css file. But we see in this example, that link in a "normal a-element" (links in JavaDoc) has a different color then a link of an "a-element with class 'header' (links in in Java method signatures that only appear when the mouse hovers over them). How should we handle these different colors? The problem also is that HTMLPrinter is located in the org.eclipse.jface.text bundle and does not know what the CSS files of the client-bundles (like JDT UI) look like. So this replacement must be quite generic and robust. The good think about a solution in HTMLPrinter would be that we only need to fix this once. If we go for a solution in the client's bundles we would fix this in all the bundles that use HTMLPrinter. Has anybody a good advice an all these questions?
(In reply to Matthias Becker from comment #3) > HTMLPrinter#convertTopLevelFont uses e.g. the regular expression: > (html\s*\{.*(?:\\s|;)font-size:\s*)\d+pt(\;?.*\}) > to e.g. replace the "9pt" in JDT's JavadocViewStyleSheet.css: > html { font-family: sans-serif; font-size: 9pt; font-style: normal; > font-weight: normal; } > with the current configured font height. > > We could do something similar with the "color" attribute of links. > Regarding links the JDT's JavadocViewStyleSheet.css contains: > > a:link { color: #0000FF; } > a:hover { color: #000080; } > a:visited { text-decoration: underline; } > a.header:link { text-decoration: none; color: InfoText } > a.header:visited { text-decoration: none; color: InfoText } > a.header:hover { text-decoration: underline; color: #000080; } You mean like insert "alinkColor", 'ahovercolor' (like InfoText) and then replace those with actual colors? If so, then I like this idea. It might be beneficial in HTMLPrinter.java:cacheColors to cache the user-preference colors and then use those in appendStyleSheets() to replaceAll alinkColor text with user colors cached in cacheColors. (Same as with Foreground/Background). > so there are 3 possible colors for links: > "link" = unvisited link > "hover" = when the user mouses over it > "visited"= visited link > > see https://www.w3schools.com/css/css_link.asp > > The "Colors and Font" preference page provides only two colors: "Basic -> > Active hyperlink text color" and "Basic -> Hyperlink text color". > How should we handle this case? Well, I don't think we have a mechanism for 'visited' links. I.e, I'm not aware that we cache visited or not-visited javadoc pages. So in such case I suggest: a:link -> Hyperlink text color a:visited -> Hyperlink text color a:hover -> Active hyperlink text color > > I fiddled around a bit with https://regex101.com/ and crafted the following > regex: (a[.[:alnum:]]*:link\s*\{.*(?:\s|;)color:\s*)[^\s\}^;]+(\;?.*\}) > that fit for the > a:link { color: #0000FF; } > and the > a.header:link { text-decoration: none; color: InfoText } > line in JDT's css file. > > But we see in this example, that link in a "normal a-element" (links in > JavaDoc) has a different color then a link of an "a-element with class > 'header' (links in in Java method signatures that only appear when the mouse > hovers over them). > How should we handle these different colors? Imho a link should always visually look like a link, so I'd say give them the same colors. I'll experiment to see how it looks like, thank you for pointing this out. > > The problem also is that HTMLPrinter is located in the > org.eclipse.jface.text bundle and does not know what the CSS files of the > client-bundles (like JDT UI) look like. So this replacement must be quite > generic and robust. The good think about a solution in HTMLPrinter would be > that we only need to fix this once. > If we go for a solution in the client's bundles we would fix this in all the > bundles that use HTMLPrinter. > > Has anybody a good advice an all these questions? Imho we should: - In HTML printer: - CacheColors() -> cache user colors for Links - in appendStyleSheet() replaceAll(aLinkColor, HyperLinkUserColor) replaceAll(aVisitedColor, ActiveHyperLinkUserColor) - update the generic .css definitions: org.eclipse.jface.internal.text.revisions.RevisionPainter.fgStyleSheet org.eclipse.ui.texteditor.RevisionHoverInformationControlCreator.fgStyleSheet and add aLinkColor, aHoverColor keywords. This would ensure proper coloring for default use cases. - Then Update the javaview and javadochover css files: /org.eclipse.jdt.ui/JavadocHoverStyleSheet.css /org.eclipse.jdt.ui/JavadocViewStyleSheet.css and similarly add aLinkColor aHoverColor - And potentially investigate loose ends where it's also listed like: eclipse.platform.releng/bundles/org.eclipse.releng.tools/book.css:a:link { color: #0000FF } egit/org.eclipse.egit.doc/help/book.css:a:link { color: #0000FF } (anybody know of any others?) If no objections, I'll work on it.
(In reply to Leo Ufimtsev from comment #4) > You mean like insert "alinkColor", 'ahovercolor' (like InfoText) and then > replace those with actual colors? > If so, then I like this idea. > It might be beneficial in HTMLPrinter.java:cacheColors to cache the > user-preference colors and then use those in appendStyleSheets() to > replaceAll alinkColor text with user colors cached in cacheColors. (Same as > with Foreground/Background). Yes. That would be a good solution. > > Well, I don't think we have a mechanism for 'visited' links. I.e, I'm not > aware that we cache visited or not-visited javadoc pages. So in such case I > suggest: > a:link -> Hyperlink text color > a:visited -> Hyperlink text color > a:hover -> Active hyperlink text color Okay with taht. > Imho a link should always visually look like a link, so I'd say give them > the same colors. > I'll experiment to see how it looks like, thank you for pointing this out. Links in method signatures are only visulized as links (get underlined and change their color to blue) when the mouse hovers over it. I think always render it as a link would reduce readability of the signatures as a lot of that text would be blue and underlined. So I think we should not change this. > Imho we should: > - In HTML printer: > - CacheColors() -> cache user colors for Links > - in appendStyleSheet() > replaceAll(aLinkColor, HyperLinkUserColor) > replaceAll(aVisitedColor, ActiveHyperLinkUserColor) > > - update the generic .css definitions: > org.eclipse.jface.internal.text.revisions.RevisionPainter.fgStyleSheet > > org.eclipse.ui.texteditor.RevisionHoverInformationControlCreator.fgStyleSheet > > and add aLinkColor, aHoverColor keywords. > This would ensure proper coloring for default use cases. > > - Then Update the javaview and javadochover css files: > /org.eclipse.jdt.ui/JavadocHoverStyleSheet.css > /org.eclipse.jdt.ui/JavadocViewStyleSheet.css > and similarly add aLinkColor aHoverColor > > - And potentially investigate loose ends where it's also listed like: > eclipse.platform.releng/bundles/org.eclipse.releng.tools/book.css:a:link > { color: #0000FF } > egit/org.eclipse.egit.doc/help/book.css:a:link { color: #0000FF } > (anybody know of any others?) > > > If no objections, I'll work on it. Okay from my side. I am willing to help with testing on macOS.
(In reply to Matthias Becker from comment #5) > Links in method signatures are only visulized as links (get underlined and > change their color to blue) when the mouse hovers over it. I think always > render it as a link would reduce readability of the signatures as a lot of > that text would be blue and underlined. So I think we should not change this. Ok, that makes sense. > > If no objections, I'll work on it. > Okay from my side. I am willing to help with testing on macOS. Kewl, I'll get to it. This bug is on my (slightly crowded) todo list. (http://bit.ly/LeosBugsTodo)
(In reply to Leo Ufimtsev from comment #6) > Kewl, I'll get to it. This bug is on my (slightly crowded) todo list. > (http://bit.ly/LeosBugsTodo) Do you think, this could be done for 4.8 M1? Hyperlinks in the Javadoc popup are not readable in the dark theme.
(In reply to Lars Vogel from comment #7) > (In reply to Leo Ufimtsev from comment #6) > > Kewl, I'll get to it. This bug is on my (slightly crowded) todo list. > > (http://bit.ly/LeosBugsTodo) > > Do you think, this could be done for 4.8 M1? Hyperlinks in the Javadoc popup > are not readable in the dark theme. Yea
Leo, today is last day for M1. You have to either fix it or move to M2.
Unfortunately I didn't get this done in time. Moving to M2.
New Gerrit change created: https://git.eclipse.org/r/117036
New Gerrit change created: https://git.eclipse.org/r/117037
(In reply to Leo Ufimtsev from comment #4) > You mean like insert "alinkColor", 'ahovercolor' (like InfoText) and then > replace those with actual colors? > If so, then I like this idea. > It might be beneficial in HTMLPrinter.java:cacheColors to cache the > user-preference colors and then use those in appendStyleSheets() to > replaceAll alinkColor text with user colors cached in cacheColors. (Same as > with Foreground/Background). My patch to HTMLPrinter does exactly this.
(In reply to Leo Ufimtsev from comment #4) > Imho we should: > - In HTML printer: > - CacheColors() -> cache user colors for Links > - in appendStyleSheet() > replaceAll(aLinkColor, HyperLinkUserColor) > replaceAll(aVisitedColor, ActiveHyperLinkUserColor) > > - update the generic .css definitions: > org.eclipse.jface.internal.text.revisions.RevisionPainter.fgStyleSheet > > org.eclipse.ui.texteditor.RevisionHoverInformationControlCreator.fgStyleSheet > > and add aLinkColor, aHoverColor keywords. > This would ensure proper coloring for default use cases. > > - Then Update the javaview and javadochover css files: > /org.eclipse.jdt.ui/JavadocHoverStyleSheet.css > /org.eclipse.jdt.ui/JavadocViewStyleSheet.css > and similarly add aLinkColor aHoverColor > > - And potentially investigate loose ends where it's also listed like: > eclipse.platform.releng/bundles/org.eclipse.releng.tools/book.css:a:link > { color: #0000FF } > egit/org.eclipse.egit.doc/help/book.css:a:link { color: #0000FF } > (anybody know of any others?) > > Adaptions to HTMLPrinter and the update the javaview and javadochover css files are done with my two gerrits. @Leo: Can you pls. review my change in plaform text? @JDT Committers: Can you pls. review my change in jdt ui? I will look into the other points once we have these two changes merged.
Created attachment 272605 [details] Before the change
Created attachment 272606 [details] After the change
Created attachment 272607 [details] After the change (light theme)
Changes to the generic .css definitions: org.eclipse.jface.internal.text.revisions.RevisionPainter.fgStyleSheet org.eclipse.ui.texteditor.RevisionHoverInformationControlCreator.fgStyleSheet are also completed.
(In reply to Matthias Becker from comment #14) > @Leo: Can you pls. review my change in plaform text? AFAIK Roland took over the color work from Leo.
(In reply to Leo Ufimtsev from comment #4) > - And potentially investigate loose ends where it's also listed like: > eclipse.platform.releng/bundles/org.eclipse.releng.tools/book.css:a:link > { color: #0000FF } > egit/org.eclipse.egit.doc/help/book.css:a:link { color: #0000FF } > (anybody know of any others?) These two places don't use HTMLPrinter. So they can stay as they are.
Gerrit change https://git.eclipse.org/r/117036 was merged to [master]. Commit: http://git.eclipse.org/c/platform/.git/commit/?id=ce56a0e12ccc495fa9319cadecf18e2bf7d6a9b6
Gerrit change https://git.eclipse.org/r/117037 was merged to []. Commit: http://git.eclipse.org/c/jdt/eclipse.git/commit/?id=f0b00f5c8b59d1db7055165562b27ab2e25a9f95
Please add to N&N
New Gerrit change created: https://git.eclipse.org/r/117155
(In reply to Eclipse Genie from comment #24) > New Gerrit change created: https://git.eclipse.org/r/117155 done. pls. review
Gerrit change https://git.eclipse.org/r/117155 was merged to [master]. Commit: http://git.eclipse.org/c/.git/commit/?id=6e84e5350f67be2bf25c8041fccb9def07215f4e
Matthias, what is the white area on the bottom and the right of the screenshot? Is that a srollbar? Looks horrible.
(In reply to Lars Vogel from comment #27) > Matthias, what is the white area on the bottom and the right of the > screenshot? Is that a srollbar? Looks horrible. Good point. Seems to be a scroolbar area (with no scrollbar). When I make the control very small a scrollbar appears. It's the same for the complete editor. That also has these white scroll bar areas.
Created attachment 272646 [details] Showing the scrollbars
(In reply to Matthias Becker from comment #29) > Created attachment 272646 [details] > Showing the scrollbars It looks like the scrollbar business is a separate issue from hyperlink text color, no? There are other factors involved, (e.g on newer gtk3/gnome3 scrollbars are hidden by default etc)... I would consider filing a separate bug for scrollbar issues.
(In reply to Leo Ufimtsev from comment #30) > (In reply to Matthias Becker from comment #29) > > Created attachment 272646 [details] > > Showing the scrollbars > > It looks like the scrollbar business is a separate issue from hyperlink text > color, no? > > There are other factors involved, (e.g on newer gtk3/gnome3 scrollbars are > hidden by default etc)... > > I would consider filing a separate bug for scrollbar issues. sure. Just wanted to answer Lars question.
(In reply to Leo Ufimtsev from comment #30) > (In reply to Matthias Becker from comment #29) > > Created attachment 272646 [details] > > Showing the scrollbars > > It looks like the scrollbar business is a separate issue from hyperlink text > color, no? > > There are other factors involved, (e.g on newer gtk3/gnome3 scrollbars are > hidden by default etc)... > > I would consider filing a separate bug for scrollbar issues. I think https://bugs.eclipse.org/bugs/show_bug.cgi?id=444560 addresses this.
New Gerrit change created: https://git.eclipse.org/r/117259
Gerrit change https://git.eclipse.org/r/117259 was merged to [master]. Commit: http://git.eclipse.org/c/www.git/commit/?id=fee573736b9438fefae427acc747a3aacc6c5612
These changes cause null pointer exceptions when used in a non-IDE environment. In particular the line at http://git.eclipse.org/c/platform/eclipse.platform.text.git/tree/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/html/HTMLPrinter.java?id=ce56a0e12ccc495fa9319cadecf18e2bf7d6a9b6#n38 will NPE, i.e., this line and likely the next line will NPE when that color isn't specified in the color registry: LINK_COLOR_RGB= JFaceColors.getHyperlinkText(display).getRGB(); ACTIVE_LINK_COLOR_RGB= JFaceColors.getActiveHyperlinkText(display).getRGB(); It would be simple to add guards... Should I open a new Bugzilla or could/should this one be used?
(In reply to Ed Merks from comment #35) > These changes cause null pointer exceptions when used in a non-IDE > environment. > > In particular the line at > http://git.eclipse.org/c/platform/eclipse.platform.text.git/tree/org.eclipse. > jface.text/src/org/eclipse/jface/internal/text/html/HTMLPrinter. > java?id=ce56a0e12ccc495fa9319cadecf18e2bf7d6a9b6#n38 will NPE, i.e., this > line and likely the next line will NPE when that color isn't specified in > the color registry: > > LINK_COLOR_RGB= JFaceColors.getHyperlinkText(display).getRGB(); > ACTIVE_LINK_COLOR_RGB= > JFaceColors.getActiveHyperlinkText(display).getRGB(); > > It would be simple to add guards... > > Should I open a new Bugzilla or could/should this one be used? From my point of view we can handle this in this bugzilla. What is different in a non-IDE environment?
The call to org.eclipse.jface.resource.JFaceColors.getHyperlinkText(Display) is implemented like this: public static Color getHyperlinkText(Display display) { return JFaceResources.getColorRegistry().get( JFacePreferences.HYPERLINK_COLOR); } The call to this registry method can (and does in my case) return null as defined in its Javadoc and as actually happens in the implemention: /** * Returns the <code>color</code> associated with the given symbolic color * name, or <code>null</code> if no such definition exists. * * @param symbolicName symbolic color name * @return the <code>Color</code> or <code>null</code> */ public Color get(String symbolicName) { Assert.isNotNull(symbolicName); Object result = stringToColor.get(symbolicName); if (result != null) { return (Color) result; } Color color = null; result = stringToRGB.get(symbolicName); if (result == null) { return null; } On the other hand, the existing implementation of JFaceColors.getInformationViewerBackgroundColor/getInformationViewerForegroundColor cannot return null because they aren't implemented to use the color registry so those existing two calls have not been a problem.
New Gerrit change created: https://git.eclipse.org/r/119501
New Gerrit change created: https://git.eclipse.org/r/119502
I pushed a fix for this to platform.text. I find it strange, that the API JFaceColors#getActiveHyperlinkText is located in org.eclipse.jface but the color definition is in org.eclipse.ui. I also updated the Java Doc of JFaceColors#getActiveHyperlinkText and JFaceColors#getHyperlinkText to inform the caller that null may be returned. I pushed this to platform.ui.
(In reply to Matthias Becker from comment #40) > I find it strange, that the API JFaceColors#getActiveHyperlinkText is > located in org.eclipse.jface but the color definition is in org.eclipse.ui. In on-IDE use cases this leads to the fact that hyperlink have the wrong color in the dark theme.
Thanks so much! I really really appreciate your responsiveness!
Gerrit change https://git.eclipse.org/r/119501 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=8b3283fcaa569f10301c31aba2924893a8a36446
Gerrit change https://git.eclipse.org/r/119502 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c9e3bdd78035336a855c94ea8047d5d21c69bd90
(In reply to Ed Merks from comment #42) > Thanks so much! I really really appreciate your responsiveness! You are welcome.
(In reply to Matthias Becker from comment #40) > I pushed a fix for this to platform.text. > > I find it strange, that the API JFaceColors#getActiveHyperlinkText is > located in org.eclipse.jface but the color definition is in org.eclipse.ui. > > I also updated the Java Doc of JFaceColors#getActiveHyperlinkText and > JFaceColors#getHyperlinkText to inform the caller that null may be returned. > I pushed this to platform.ui. Does anybody have some details for that?
*** Bug 531534 has been marked as a duplicate of this bug. ***
