Bug 152128

Summary: Color for highlighted replace matches is disabled, but should be active
Product: [Unmaintained] kdelibs Reporter: Thomas McGuire <mcguire>
Component: generalAssignee: kdelibs bugs <kdelibs-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: kstars, mwoehlke.floss
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: patch to change inactive selection colors to be a bit more selection-like

Description Thomas McGuire 2007-11-10 19:03:28 UTC
Version:           SVN trunk (using KDE Devel)
Installed from:    Compiled sources
OS:                Linux

When doing a search&replace with enabled "Prompt on replace", a dialog pops up asking the user if he really wants to replace the highlighted word.
For this, of course, the word is highlighted in the textedit.
However, it is highlighted in gray, but instead it should be highlighted in blue, so that the user actually sees at first glance which word in his text is highlighted.

The reason is that the replace prompt dialog is active, and therefore the textedit is disabled, and gets a disabled color palette. This clearly doesn't work out in this case.

You can try it in KMail's composer.
Comment 1 Thomas McGuire 2007-11-13 19:21:49 UTC
Same issue with the folder select dialog: Clicking on a folder on the left pane highlights the folder in the tree view. However, this is difficult to see as the highlight color is wrong, it is gray instead of blue.

Do we really need more examples? This color palette changing is broken in many cases and should be switched off by default.
Comment 2 Matthew Woehlke 2007-11-13 22:51:01 UTC
Care to offer a better solution that doesn't throw out the good with the bad?
Comment 3 Thomas McGuire 2007-11-14 23:47:59 UTC
>Care to offer a better solution that doesn't throw out the good with the bad? 
Sorry, I don't have any better solution, I don't know any of the color code at all. I don't even know if it is possible to fix these things properly, maybe it is impossible because of the general design/idea; but then again, I know nothing about it.

Personally, I just want the palette bugs to go away, one way or the other.
Comment 4 kstars 2007-11-16 15:53:51 UTC
The color palette should not be "disabled" just because the widget loses input focus.  "disabled" means "you cannot interact with this widget", it doesn't mean "you are not interacting with this widget at the moment".  

This feature causes huge usability problems; I have a dialog with a QTableView and other widgets; if the table loses input focus to another widget in the dialog, it is not possible to see the selection anymore!  See here for screenshots:
http://www.kdedevelopers.org/node/3103
Comment 5 Andreas Pakulat 2007-11-16 16:01:36 UTC
I may be wrong, but isn't the problem that the treewidget and possibly also the textedit use the "disabled" role, when instead they should use "inactive"? I think this is a bug in Qt's QTreeView and whatever textedit has the prompt on replace stuff (KTextEdit or Kate I guess).
Comment 6 kstars 2007-11-16 17:47:49 UTC
FWIW, I am seeing the problem with a QTableView widget (see the sreenshots at the link provided in comment #4).
Comment 7 Kevin Kofler 2007-11-17 06:45:05 UTC
FWIW, I added this hack to the Quarticurve style on October 26 exactly to override this behavior (actually, I override the color even for "disabled" because I want it to look like Bluecurve for Qt 3 and GTK+ 2, which shows the selection in blue even for disabled controls):

static QPalette adjustPalette(QPalette p)
{
    /* Force the selection colors to always active. This overrides KDE 4's
       behavior of using window colors for the inactive or disabled
       selection and matches the KDE 3 and GTK+ themes. */
    QBrush highlight = p.brush(QPalette::Active, QPalette::Highlight);
    p.setBrush(QPalette::Inactive, QPalette::Highlight, highlight);
    p.setBrush(QPalette::Disabled, QPalette::Highlight, highlight);
    QBrush highlightedText = p.brush(QPalette::Active, QPalette::HighlightedText);
    p.setBrush(QPalette::Inactive, QPalette::HighlightedText, highlightedText);
    p.setBrush(QPalette::Disabled, QPalette::HighlightedText, highlightedText);
    return p;
}

(Then the first thing all the draw* functions do is calling this on the palette they receive.)
Comment 8 Thomas McGuire 2007-11-18 15:16:47 UTC
>I may be wrong, but isn't the problem that the treewidget and possibly also the textedit use the "disabled" role, when instead they should use "inactive"? I think this is a bug in Qt's QTreeView and whatever textedit has the prompt on replace stuff (KTextEdit or Kate I guess). 
I guess that you could be right and that there are indeed tons of palette bugs in all kind of widgets. Then we have to options:

1) Fix all the widget bugs (some widgets are in Qt, so this is a big problem)
2) Disable the color palette changing, as it is too buggy
 
Honestly I don't think 1) is easily doable. At this point, we're near a release, and I don't want to spend my time hunting palette bugs. If someone else does it, fine, but I'm not going to do it.
We should just disable the stuff by default.

I also discovered a new occurrence of the bug:
In the KMail settings dialog, when you click on "Appearance" the first time, it looks gray, and when you click the second time, it is blue.
Again, this may be a widget bug.
Comment 9 Matthew Woehlke 2007-11-19 17:56:37 UTC
Ok, I have an idea for making inactive look less "disabled" (I still don't get the "disabled" thing... it's not disabled at all, it's using the Window colors). I'm hoping to get it coded tonight, but not sure how long it will take; KColorScheme isn't really written to have a wholly different color set for one set/state combination.

I'm just paranoid that any solution other than the current behavior (or the old, "different than the rest of the world" behavior) is 100% palette-safe :-(.
Comment 10 Thomas McGuire 2007-11-19 18:14:29 UTC
OK, I'll test the patch when it is available.
Comment 11 Matthew Woehlke 2007-11-20 02:12:54 UTC
Created attachment 22128 [details]
patch to change inactive selection colors to be a bit more selection-like

Ok, all, take this for a spin... I'm attaching a patch that adds some tint from
active Selection:NormalBackground into the inactive (but *not* the disabled)
thereof. So, disabled selections will use Window colors straight (I think this
is still OK?), but inactive selections will use Window colors but with the
background tinted toward active Selection:NormalBackground.

This should only cause Usability/Accessibility problems if a Window foreground
color has low luma contrast w.r.t. its corresponding background color.

I think this works better than fredrikh's suggestion of doing the opposite
(still use Selection, but tint with Window:NormalBackground) as I think the
problem is more than inactive selection needs to be more selection hue+chroma
with different luma than window hue+chrome with same luma. (This doesn't work
as well with schemes like BeOS where the selection color does not differ in
hue/chroma, but the alternative would result in no distinction between active
and inactive, which I consider very undesirable.)
Comment 12 Thomas McGuire 2007-11-21 17:34:34 UTC
OK, I tried the patch and it is definitely an improvement, thanks.
Please commit this.

I still would like an option in the color KCM to disable palette changing completely (even though it is subtle now, it is still there).

But if the patch gets commited, I'm also happy. 
Comment 13 Matthew Woehlke 2007-11-21 18:14:56 UTC
SVN commit 739713 by mwoehlke:

Tint inactive selection (Window colors) with Active:Selection:NormalBackground so that it looks more like a selection (and is less likely, we hope, to be indistinguishable from View:AlternateBackground). This hopefully also allows hacks to use Active:Selection instead of Inactive:Selection in widgets using Window and not View to be removed.
BUG: 152128


 M  +44 -4     kcolorscheme.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=739713