Community
Participate
Working Groups
Created attachment 227167 [details] Screenshot with showing hyperlinks I'm unable to style with css. I'm currently trying to style Eclipse with a Dark theme (by that I mean I want a dark theme for everything), but it seems it's not possible to style a hyperlink within a Link widget (which makes it difficult to use Eclipse with a dark theme). I'm adding a screenshot to show what I mean by that if it didn't become clear. This is on Eclipse 4.3m5 (Build id: I20130204-1400)
I'm really worried about this issue -- I'm working on styling a completely dark IDE with Eclipse and this is the main thing missing and I'm not confident enough to provide a patch for this right now without more input from someone more familiar with the area. Apparently the issue is on swt (which is system-dependent, so, I'm only talking about windows here): the Link widgets seems to be created without any possible customization of the hyperlink color. i.e.: so, here is the first point: at: org.eclipse.swt.widgets.Link.createHandle(): on newer versions of windows, it uses the system and doesn't even use its linkColor / disabledColor. void createHandle () { super.createHandle (); state |= THEME_BACKGROUND; if (OS.COMCTL32_MAJOR < 6) { layout = new TextLayout (display); if (!OS.IsWinCE && OS.WIN32_VERSION >= OS.VERSION (4, 10)) { linkColor = Color.win32_new (display, OS.GetSysColor (OS.COLOR_HOTLIGHT)); } else { linkColor = new Color (display, LINK_FOREGROUND); } disabledColor = Color.win32_new (display, OS.GetSysColor (OS.COLOR_GRAYTEXT)); offsets = new Point [0]; ids = new String [0]; mnemonics = new int [0]; selection = new Point (-1, -1); focusIndex = mouseDownIndex = -1; } } and later on at the org.eclipse.swt.widgets.Link.drawWidget(GC, RECT), the same thing happens: LRESULT WM_PAINT (int /*long*/ wParam, int /*long*/ lParam) { if (OS.COMCTL32_MAJOR >= 6) { return super.WM_PAINT (wParam, lParam); } PAINTSTRUCT ps = new PAINTSTRUCT (); GCData data = new GCData (); data.ps = ps; data.hwnd = handle; GC gc = new_GC (data); if (gc != null) { int width = ps.right - ps.left; int height = ps.bottom - ps.top; if (width != 0 && height != 0) { RECT rect = new RECT (); OS.SetRect (rect, ps.left, ps.top, ps.right, ps.bottom); drawWidget (gc, rect); } gc.dispose (); } return LRESULT.ZERO; } so, as the system would do the drawing, we currently have zero customization options here (and thus no themeing can be used). An option would be to revert the optimization in newer systems and always revert to do the drawing ourselves with a StyledText (and create setHyperlinkColor/setDisabledHyperlinkColor methods which would allow us to change the color) -- or maybe the system allows a way to set the handle color for links through a different way which we could customize? Also other OS implementations would have to be checked too.
*** Bug 430993 has been marked as a duplicate of this bug. ***
*** Bug 436899 has been marked as a duplicate of this bug. ***
Created attachment 254690 [details] Quick fix dialog on the dark theme on MacOS
I am having the same issue on MacOS with Eclipse Mars. It also occurs in the popup which is shown when hovering over an error. This popup shows quick fixes which are unreadable because of the color. I added a screenshot of the popup.
Is this issue fixed already? Or do someone have a solution to solve this problem? Thx
It's not fixed on my mac running Mars.1. Still completely illegible and no matter what setting I change the colors remain the same as attachment 254690 [details]. Could really use a fix for this. I'm new to Java programming and these tooltips would be really nice... if I could read them. I'm running OS X Mavericks and Eclipse Mars.1
I worked on this bugs for a few days now and it is easily possible, to enhance CSS support for hyperlinks by using the selector :link (see http://www.w3schools.com/cssref/css_selectors.asp). The first gerrit change contains a very simple implementation draft, that is able to set a link color, which is currently not supported by the Link widget. The problem is (at least for newer Windows versions with OS.COMCTL32_MAJOR >= 6) the SysLink control is used and that control either uses the system-defined color for hyperlinks or the text color, which looks like attachment "link with text color" (source: https://social.msdn.microsoft.com/Forums/en-US/bd54bd30-e21f-4dc7-a77f-88de02c63f72/action?threadDisplayName=vstudio&forum=vcgeneral). Using the text color can be enabled with a simple state mask (see second gerrit change). If we want to style the link-parts of the Link widget separately, we need to implement custom drawing functionality for Windows. I had a shor look into the cocoa and gtk Link widgets and it seams for me, that it is much easier, to implement a link color there. But before I spend some more hours of work on this, I want to know, if I'm on the right way, especially if I add API to the Link widget like setLinkColor.
Created attachment 258957 [details] link with text color, disabled Link widget and a Text widget
New Gerrit change created: https://git.eclipse.org/r/63416
New Gerrit change created: https://git.eclipse.org/r/63417
An alternative for setLinkColor would be the existing method setData (String key, Object value) with e.g. key="linkColor".
(In reply to Conrad Groth from comment #12) > An alternative for setLinkColor would be the existing method setData (String > key, Object value) with e.g. key="linkColor". Hi Conrad, New API/method not needed here.. we should override Control#setForeground(Color) in Link class which should be able to change the color of Link text.
(In reply to Niraj Modi from comment #13) > New API/method not needed here.. we should override > Control#setForeground(Color) in Link class which should be able to change > the color of Link text. This bug is about the text within a link vs outside of the link. For example, consider the following text: "The quick brown <a>fox jumped over</a> the lazy dog", then setForeground() should affect the colour of "The quick brown " and " the lazy dog", and setLinkColor() would affect the colour of "fox jumped over".
(In reply to Brian de Alwis from comment #14) > (In reply to Niraj Modi from comment #13) > > New API/method not needed here.. we should override > > Control#setForeground(Color) in Link class which should be able to change > > the color of Link text. > > This bug is about the text within a link vs outside of the link. For > example, consider the following text: "The quick brown <a>fox jumped > over</a> the lazy dog", then setForeground() should affect the colour of > "The quick brown " and " the lazy dog", and setLinkColor() would affect the > colour of "fox jumped over". Giving the hyperlink AND the surrounding text the same color, is the easiest solution, especially on windows. But I think, the intention of the bug author was to color the hyperlink part different than the text part of the Link widget.
(In reply to Conrad Groth from comment #15) > (In reply to Brian de Alwis from comment #14) > > (In reply to Niraj Modi from comment #13) > > > New API/method not needed here.. we should override > > > Control#setForeground(Color) in Link class which should be able to change > > > the color of Link text. > > > > This bug is about the text within a link vs outside of the link. For > > example, consider the following text: "The quick brown <a>fox jumped > > over</a> the lazy dog", then setForeground() should affect the colour of > > "The quick brown " and " the lazy dog", and setLinkColor() would affect the > > colour of "fox jumped over". > > Giving the hyperlink AND the surrounding text the same color, is the easiest > solution, especially on windows. But I think, the intention of the bug > author was to color the hyperlink part different than the text part of the > Link widget. setForeground() does not change the color of the link text. We shouldn't change this API behavior. Instead we should have a new API to set the link text color independent of the text color of the non-link text. Something like setLinkForeground() on the same lines of setForeground()?
Yeah, I'm finished! At least until someone reviews the SECOND gerrit change ;) Like I mentioned in comment 8, windows was a pain. The SysLink control has a limited API, which either allows a state flag, to color the link parts with the same color than the surrounding text, or to use the visual style defined by the current OS theme. This OS theme provides two colors for the SysLink control, one for the text and one for the hyperlink(s). Changing the OS theme from an application seemed for me as no proper solution. I don’t even know, if it’s possible. So I used the already present code for drawing the Link myself, if the linkForeground was defined by the user. I know, that the gtk and cocoa implementations are compiling. Can someone please test that on these platforms? Added new API for the Link widget on all platforms: + setLinkForeground(Color color) to set the color for the hyperlink parts of the Link widget. + getLinkForeground() returns the color, that was set by setLinkForeground. It does NOT return the default link color, because at least on some windows versions (OS.COMCTL_MAJOR >= 6) this color cannot be determined so easily. The first gerrit change needs to be updated, after the Link widget provides the new API.
Setting target to M6 as it is API freeze. I've added some review comments to the gerrit patch.
I provided an update for the cocoa Link widget. I'm working on windows and gtk right now. During implementation of the review comments I realized, that setLinkForeground and getLinkForeground don't respect the enabled state. In fact, the mentioned getter and setter implicitely assumed an enabled state. Although it was just a small step for me, I think it is a big improvement for SWT.
(In reply to Conrad Groth from comment #19) > During implementation of the review comments I realized, that > setLinkForeground and getLinkForeground don't respect the enabled state. In > fact, the mentioned getter and setter implicitely assumed an enabled state. All the setForeground() and getForeground() APIs in SWT return values for the enabled state. The new APIs for Link should be consistent with the existing SWT APIs. We shouldn't provide an API to set the disabled link text color. When the Link widget is disabled all the text including the link should use the system default color for the disabled state. Having the link text alone in any other color would give an impression that it's not disabled.
(In reply to Lakshmi Shanmugam from comment #20) > (In reply to Conrad Groth from comment #19) > > > During implementation of the review comments I realized, that > > setLinkForeground and getLinkForeground don't respect the enabled state. In > > fact, the mentioned getter and setter implicitely assumed an enabled state. > All the setForeground() and getForeground() APIs in SWT return values for > the enabled state. The new APIs for Link should be consistent with the > existing SWT APIs. > We shouldn't provide an API to set the disabled link text color. When the > Link widget is disabled all the text including the link should use the > system default color for the disabled state. Having the link text alone in > any other color would give an impression that it's not disabled. +1. Plus, all APIs in SWT are get¦set/Fore¦Back/ground. There is e.g. no getEnabledLabelForeground.
I updated all three implementations of the Link widget, so that getLinkForeground always returns a color, either the user defined or the system specific one. I also removed the enabled parameter from setLinkForeground and getLinkForeground.
What a blooper, I updated the GTK implementation.
Conrad, can you please add unit tests for testing the new Link API?
I added a test for the new API, but unfortunately now the build fails, because it uses the exisisting Link widget, which doesn't provide the new API. I don't know, how to solve this without deleting the test again. Maybe the change gets merged in the near future without a succesful build.
(In reply to Conrad Groth from comment #25) > I added a test for the new API, but unfortunately now the build fails, > because it uses the exisisting Link widget, which doesn't provide the new > API. I don't know, how to solve this without deleting the test again. Maybe > the change gets merged in the near future without a succesful build. That is correct, one workaround for this was to push the API changes first and then create a separate gerrit patch just for the addition of the unit test... However, I've gone ahead and pushed the gerrit patch directly from my workspace to avoid further delays (via commit http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=32794db089ad0fe52c5aa02fdea493f02952d274).
Looking this addition (on win32 systems): +public Color getLinkForeground () { + checkWidget (); + return internalGetLinkForeground(); +} + +Color internalGetLinkForeground() { + if (linkForeground != -1) { + return Color.win32_new (display, linkForeground); + } + if (!OS.IsWinCE && OS.WIN32_VERSION >= OS.VERSION (4, 10)) { + return Color.win32_new (display, OS.GetSysColor (OS.COLOR_HOTLIGHT)); + } + return new Color (display, LAST_FALLBACK_LINK_FOREGROUND); +} I would argue that if the last line is entered, a resource leak will be provoked, since each time when the public function getLinkForeground will be called, a new Color instance is created.
(In reply to Daniel Kruegler from comment #27) > Looking this addition (on win32 systems): > > +public Color getLinkForeground () { > + checkWidget (); > + return internalGetLinkForeground(); > +} > + > +Color internalGetLinkForeground() { > + if (linkForeground != -1) { > + return Color.win32_new (display, linkForeground); > + } > + if (!OS.IsWinCE && OS.WIN32_VERSION >= OS.VERSION (4, 10)) { > + return Color.win32_new (display, OS.GetSysColor (OS.COLOR_HOTLIGHT)); > + } > + return new Color (display, LAST_FALLBACK_LINK_FOREGROUND); > +} > > I would argue that if the last line is entered, a resource leak will be > provoked, since each time when the public function getLinkForeground will be > called, a new Color instance is created. Yes, it looks a resource leak scenario.. I share a gerrit shortly.
New Gerrit change created: https://git.eclipse.org/r/68378
Gerrit change https://git.eclipse.org/r/68378 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=0e4b82944684ce982f4b52ddb55fb1c6fe794ffe
(In reply to Eclipse Genie from comment #30) > Gerrit change https://git.eclipse.org/r/68378 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=0e4b82944684ce982f4b52ddb55fb1c6fe794ffe Above patch fixes the possible resource leak scenario, mentioned in comment 27. Resolving now.
Verified on Cocoa with I20160314-2000.
Verified fix on Win32 with I20160314-2000.
