Skip to content

Partial Revert 'Compute foreground and background'#3209

Closed
elsazac wants to merge 1 commit intoeclipse-platform:masterfrom
elsazac:StyledTextEditor
Closed

Partial Revert 'Compute foreground and background'#3209
elsazac wants to merge 1 commit intoeclipse-platform:masterfrom
elsazac:StyledTextEditor

Conversation

@elsazac
Copy link
Copy Markdown
Member

@elsazac elsazac commented Apr 9, 2026

This commits partially reverts changes for Background color when editable was false

Fixes: #3207

@BeckerWdf
Copy link
Copy Markdown
Member

@HeikoKlare: Can you verify if this PR fixes the regression?

This commits partially reverts changes for Background color when
editable was false

Fixes: eclipse-platform#3207
@elsazac elsazac force-pushed the StyledTextEditor branch from 9a70719 to e4b685d Compare April 9, 2026 11:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

Test Results

  170 files    170 suites   28m 2s ⏱️
4 679 tests 4 658 ✅  21 💤 0 ❌
6 592 runs  6 437 ✅ 155 💤 0 ❌

Results for commit e4b685d.

@HeikoKlare
Copy link
Copy Markdown
Contributor

The background is definitely better with this PR, but the color is still different than before. I am not sure if this is intended. It would somehow make sense to have a grayish background in read-only documents whereas it was just white before, so might be an intended change.

Old:
image

New:
image

@akurtakov
Copy link
Copy Markdown
Member

This also breaks cursor (totally invisible) on Gtk 4. With #3208 Cursor draws just fine. I'm closing this one and pushing #3208 to get us back to working state.

@akurtakov akurtakov closed this Apr 9, 2026
@iloveeclipse
Copy link
Copy Markdown
Member

The background is definitely better with this PR, but the color is still different than before. I am not sure if this is intended.

This is probably not intended, at least it would make every "read only" text not as readable as before. Also I imagine every product has its own definition of what is supposed to be shown in the "read only" editors, and this would most likely not wanted (I personally would not accept such change if I would have editor in a product that has white background before).

@elsazac
Copy link
Copy Markdown
Member Author

elsazac commented Apr 9, 2026

It would somehow make sense to have a grayish background in read-only documents whereas it was just white before, so might be an intended change.

This was an intended change. The editable case wasn't distinguishable from non editable one, hence given a different background for the non editable behaviour.

@iloveeclipse
Copy link
Copy Markdown
Member

This was an intended change. The editable case wasn't distinguishable from non editable one, hence given a different background for the non editable behaviour.

So all Java sources from dependencies should appear with a grey background? Please don't do that by default.

If something like this should be done, then only if explicitly requested by the provider of the particular tooling (like JDT should decide whether read only Java sources should have different background by default, or EGit should decide if diff editor should show gray background by default for read only side of the diff) etc.

In most cases in the "default" IDE I personally would never want to see grey background for "read only" text files (independently if it is diff or regular editor), simply because it would greatly affect readability of the text for me. So if something like this should be provided, it should be configurable by a preference.

@akurtakov
Copy link
Copy Markdown
Member

If one runs SWT's CustomControlExample and check the SWT.READ_ONLY - StyledText will become gray (but lighter on GTK )
image
So it is already the case for StyledText - what Eclipse Text or JDT decided to override after that seems to be another story .

@iloveeclipse
Copy link
Copy Markdown
Member

So it is already the case for StyledText - what Eclipse Text or JDT decided to override after that seems to be another story .

Obviously exact that was broken by #3154.

@elsazac
Copy link
Copy Markdown
Member Author

elsazac commented Apr 9, 2026

If one runs SWT's CustomControlExample and check the SWT.READ_ONLY - StyledText will become gray (but lighter on GTK )

I am not sure on how this appears in other systems but in Mac it appears to be a clean background in read-only, even with new proposed changes #3209

Screenshot 2026-04-09 at 7 59 37 PM

@elsazac
Copy link
Copy Markdown
Member Author

elsazac commented Apr 9, 2026

and regarding how it appears on a class file or compare editor both being non editable, this is the behaviour I see in Mac.

The initial intend was to have a greyish tint for non editable, which caused the regression #3207 in light theme,I agree, but this one #3209 revert the changes done for editable, so ideally it get backs the class file or compare editor to the pervious behaviour.

image image

@elsazac
Copy link
Copy Markdown
Member Author

elsazac commented Apr 10, 2026

This also breaks cursor (totally invisible) on Gtk 4.

@akurtakov Just curious, were you able to test the cursor behaviour in gtk4 with this current patch.

@akurtakov
Copy link
Copy Markdown
Member

@akurtakov Just curious, were you able to test the cursor behaviour in gtk4 with this current patch.

Yes, I tested with it and confirmed this was not helping at all before closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The wrong color palette is used for several editors

5 participants