Bug 163335 - [Themes] JFaceColors.getHyperlinkText() returns incorrect color in high contrast mode
Summary: [Themes] JFaceColors.getHyperlinkText() returns incorrect color in high contr...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows Vista
: P3 major (vote)
Target Milestone: 3.8.2+   Edit
Assignee: Paul Webster CLA
QA Contact: Susan McCourt CLA
URL:
Whiteboard:
Keywords: accessibility
: 57044 89373 176381 313396 (view as bug list)
Depends on: 181592
Blocks: 115863
  Show dependency tree
 
Reported: 2006-11-03 10:15 EST by Tod Creasey CLA
Modified: 2013-04-01 10:56 EDT (History)
13 users (show)

See Also:


Attachments
screenshot (19.30 KB, image/png)
2006-11-03 10:16 EST, Tod Creasey CLA
no flags Details
Patch (4.26 KB, patch)
2013-02-12 16:57 EST, Bogdan Gheorghe CLA
no flags Details | Diff
Patch (4.34 KB, patch)
2013-02-12 16:59 EST, Bogdan Gheorghe CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tod Creasey CLA 2006-11-03 10:15:53 EST
Version: 3.3.0
Build id: I20061102-0010

The Help fonts and colours appear to be hardcoded to blue so as a result are unreadable - they should use the constants for active and hypperlink colour.

STEPS
1) Turn on High contrast
2) Open help - unreadable
Comment 1 Tod Creasey CLA 2006-11-03 10:16:50 EST
Created attachment 53198 [details]
screenshot
Comment 2 Dejan Glozic CLA 2006-11-03 11:16:50 EST
Did you restart Eclipse? If you do I think help will pick up the new colors as defined in JFaceResources.
Comment 3 Curtis d'Entremont CLA 2006-11-03 11:22:43 EST
I tried it but it didn't help.. still blue. This is not just help but seems to be all forms.
Comment 4 Tod Creasey CLA 2006-11-03 11:27:56 EST
When you switch to High Contrast you get prompted to restart which I did.
Comment 5 Dejan Glozic CLA 2006-11-03 11:33:37 EST
Last time I checked, Forms use normal and active hyperlink colors from JFaceResources or some such. Did these values change after restart?
Comment 6 Tod Creasey CLA 2006-11-03 11:58:19 EST
Yes - when you restart due to High Contrast change the themes change
Comment 7 Curtis d'Entremont CLA 2006-11-03 12:52:08 EST
JFace is still giving {0, 0, 128} even after restarting the workbench after switching to high contrast.
Comment 8 Tod Creasey CLA 2006-11-13 14:46:37 EST
This is working if you completely shutdown and then restart but not with the High Contrast change restart
Comment 9 Curtis d'Entremont CLA 2007-03-05 16:20:25 EST
*** Bug 176381 has been marked as a duplicate of this bug. ***
Comment 10 Michael Burkhart CLA 2007-03-23 08:31:18 EDT
I would like to request that the severity of this bug be raised to Major since it is considered an accessibility issue for our product release.
Comment 11 Kim Horne CLA 2007-03-23 09:00:01 EDT
Am I understanding that this issue only presents itself immediately after a change into high contrast that isn't followed by a reboot?  If this is the case I dont think it's too big a deal - people who need high contrast support wont just be switching into it, they'll be running with it consistently.
Comment 12 Michael Burkhart CLA 2007-03-23 09:09:19 EDT
The bug record (Bug 176381) that was dup'd against this one does not have that limitation in our testing so either it should not have been dup'd or we are experiencing different results in our testing. I was able to reproduce this consistently with eclipse 3.3 by starting in HC mode and then launching eclipse.
Comment 13 Mike Wilson CLA 2007-03-23 14:43:06 EDT
Can we get the colors by creating just a link widget and asking it for it's foreground and background?
Comment 14 Tod Creasey CLA 2007-04-09 09:34:51 EDT
We will need support from SWT to be able to access these colors as they are currently not available through API.

See Bug 181592
Comment 15 Tod Creasey CLA 2007-04-13 16:16:54 EDT
*** Bug 89373 has been marked as a duplicate of this bug. ***
Comment 16 Tod Creasey CLA 2007-04-18 16:27:56 EDT
Marking for 3.4. We will have to see what SWT can do about Bug 181592 before we can proceed on this.
Comment 17 Mike Wilson CLA 2007-06-05 12:02:50 EDT
Lowering priority to P3. This is a real bug, but it's not going to stop us from shipping. Changing severity to Major to capture the importance to the reporters.
Comment 18 Tod Creasey CLA 2007-06-20 15:32:45 EDT
*** Bug 57044 has been marked as a duplicate of this bug. ***
Comment 19 Tod Creasey CLA 2008-01-28 09:53:40 EST
Removing the 3.4 tag as we need Bug 181592 to be resolved before we can move forward
Comment 20 Boris Bokowski CLA 2009-11-17 11:33:47 EST
Susan is now responsible for watching the [Themes] category.
Comment 21 Bogdan Gheorghe CLA 2013-02-12 12:56:51 EST
Here is a snippet for calculating a second color that's darker from some original color:

float scaleFactor = 0.7f;
RGB rgb = new RGB(0, 102, 204);
float[] hsb = rgb.getHSB();
hsb[2] *= scaleFactor;
if (hsb[2] < 0) hsb[2] = 0;
if (hsb[2] > 255)  hsb[2] = 255;
Color newColor = new Color(display, new RGB(hsb[0], hsb[1], hsb[2]));
Comment 22 Bogdan Gheorghe CLA 2013-02-12 16:57:15 EST
Created attachment 226966 [details]
Patch

Here is a patch that will change the default JFace hyperlink color preferences to be keyed off the system hyperlink preference. This is a good thing as it means the default will now respect high contrast mode.

It's easy to verify this on Windows: open the Help view, notice the link colors and then change to High Contrast mode.
Comment 23 Bogdan Gheorghe CLA 2013-02-12 16:59:50 EST
Created attachment 226967 [details]
Patch

Updated the comments in the patch.
Comment 24 Paul Webster CLA 2013-02-13 14:30:30 EST
(In reply to comment #23)
> Created attachment 226967 [details]
> Patch

Looks good.

1) could we switch the factory to ACTIVE_HYPERLINK_COLOR instead of HYPERLINK_COLOR?  I don't mind if the active link is slightly lighter based on the factory.

2) Could we update org.eclipse.ui.forms.HyperlinkSettings.initializeDefaultForegrounds(Display) (in eclipse.platform.ua) so it uses COLOR_LINK_FOREGROUND as the fail-through default

3) we need to update o.e.ui.workbench and o.e.ui.forms to import an SWT of 3.101 (or whatever the 4.3 stream SWT is) since we use a new API constant)

PW
Comment 25 Paul Webster CLA 2013-02-13 15:43:51 EST
Thanks Bogdan, and sorry I forgot to swap the author.

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a6d95ae11b9b4967c998ffe8ad15fa43591e8034

PW
Comment 26 Dani Megert CLA 2013-02-14 05:47:25 EST
*** Bug 313396 has been marked as a duplicate of this bug. ***
Comment 27 Markus Keller CLA 2013-02-14 08:32:46 EST
The "if (hsb[2] > 255) hsb[2] = 255;" in RGBBrightnessColorFactory doesn't make sense.

Fixed this bug, removed the harmful error swallowing and other bad coding style, and formatted the code:
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ee57e66cde7637caf434dc4cbaa5b1362f022c85
Comment 28 Carolyn MacLeod CLA 2013-03-21 16:55:18 EDT
Markus or Paul, would you be able to please backport this to R4_2_maintenance
(and, if applicable, R3_8_maintenance)? All SWT API changes in bug 181592 have now been backported to R4_2_maintenance.