bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[Juanma Barranquero]

Package: emacs,w32
Version: 23.0.60

default-frame-alist affects tooltips on Windows, but the background of
the tooltip window is not affected.

With

(setq default-frame-alist
      '((background-mode  . dark)
	(background-color . "black")
	(foreground-color . "white")))

the tooltip text foreground is white, but the background is still the
default lightyellow, so the text is not visible.

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

> With
> 
> (setq default-frame-alist
>       '((background-mode  . dark)
> 	(background-color . "black")
> 	(foreground-color . "white")))
> 
> the tooltip text foreground is white, but the background is still the
> default lightyellow, so the text is not visible.

Does the attached patch fix it?

martin


[-- Attachment #2: 1032.diff --]
[-- Type: text/plain, Size: 1423 bytes --]

*** w32fns.c.~1.347.~	2008-09-19 23:24:49.062500000 +0200
--- w32fns.c	2008-09-26 17:10:10.953125000 +0200
***************
*** 5532,5546 ****
       attribute of the frame get's set, which let's the internal border
       of the tooltip frame appear in pink.  Prevent this.  */
    {
      Lisp_Object bg = Fframe_parameter (frame, Qbackground_color);
  
      /* Set tip_frame here, so that */
      tip_frame = frame;
      call2 (Qface_set_after_frame_default, frame, Qnil);
  
!     if (!EQ (bg, Fframe_parameter (frame, Qbackground_color)))
!       Fmodify_frame_parameters (frame, Fcons (Fcons (Qbackground_color, bg),
! 					      Qnil));
    }
  
    f->no_split = 1;
--- 5532,5549 ----
       attribute of the frame get's set, which let's the internal border
       of the tooltip frame appear in pink.  Prevent this.  */
    {
+     Lisp_Object fg = Fframe_parameter (frame, Qforeground_color);
      Lisp_Object bg = Fframe_parameter (frame, Qbackground_color);
  
      /* Set tip_frame here, so that */
      tip_frame = frame;
      call2 (Qface_set_after_frame_default, frame, Qnil);
  
!     if (!EQ (bg, Fframe_parameter (frame, Qbackground_color))
! 	|| !EQ (fg, Fframe_parameter (frame, Qforeground_color)))
! 	Fmodify_frame_parameters (frame, (Fcons (Qforeground_color, fg),
! 					  (Fcons (Qbackground_color, bg),
! 					   Qnil)));
    }
  
    f->no_split = 1;

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

On Fri, Sep 26, 2008 at 17:14, martin rudalics <rudalics@gmx.at> wrote:

> Does the attached patch fix it?

Yes, in the sense that now the text is readable: the whole tooltip is
white on black, as it is the Emacs frame, so the text is legible but
the tooltip is hard to tell apart from the background (see attached
image).

I think default frame settings should not affect tooltips (perhaps
some mechanism like scroll-bar-{foreground|background} ?).

             Juanma

[-- Attachment #2: sample.png --]
[-- Type: image/png, Size: 9814 bytes --]

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[martin rudalics]

 > Yes, in the sense that now the text is readable: the whole tooltip is
 > white on black, as it is the Emacs frame, so the text is legible but
 > the tooltip is hard to tell apart from the background (see attached
 > image).

I noticed that too.  It's because border-color is not set as well.

 > I think default frame settings should not affect tooltips (perhaps
 > some mechanism like scroll-bar-{foreground|background} ?).

Can you produce a good tooltip by setting `tooltip-frame-parameters'
appropriately?  Here I can't.  For example, the following gets always
reset as

(background-color . "systeminfowindow")

martin

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[Juanma Barranquero]

On Fri, Sep 26, 2008 at 18:13, martin rudalics <rudalics@gmx.at> wrote:

> Can you produce a good tooltip by setting `tooltip-frame-parameters'
> appropriately?  Here I can't.  For example, the following gets always
> reset as
>
> (background-color . "systeminfowindow")

Same here:

*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> tooltip-frame-parameters
((name . "tooltip")
 (internal-border-width . 2)
 (border-width . 1))

ELISP> (setq tooltip-frame-parameters (append '((background-color .
"lightyellow") (foreground-color . "black"))
tooltip-frame-parameters))
((background-color . "lightyellow")
 (foreground-color . "black")
 (name . "tooltip")
 (internal-border-width . 2)
 (border-width . 1))

;;; Show tooltip, then:

ELISP> tooltip-frame-parameters
((background-color . "systeminfowindow")
 (foreground-color . "systeminfotext")
 (name . "tooltip")
 (internal-border-width . 2)
 (border-width . 1))

             Juanma

bug#1032: marked as done (foreground-color in default-frame-alist affects tooltips, while background-color does not)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]


Your message dated Thu, 30 Oct 2008 02:43:27 +0100
with message-id <f7ccd24b0810291843l2edc56fbx3b275a029ba0ee81@mail.gmail.com>
and subject line Re: bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not
has caused the Emacs bug report #1032,
regarding foreground-color in default-frame-alist affects tooltips, while background-color does not
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
1032: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1032
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3418 bytes --]

From: "Juanma Barranquero" <lekktu@gmail.com>
To: Bug-Gnu-Emacs <bug-gnu-emacs@gnu.org>
Subject: foreground-color in default-frame-alist affects tooltips, while background-color does not
Date: Fri, 26 Sep 2008 10:40:10 +0200
Message-ID: <f7ccd24b0809260140o24250681w94a6e4dfae5d49c7@mail.gmail.com>

Package: emacs,w32
Version: 23.0.60

default-frame-alist affects tooltips on Windows, but the background of
the tooltip window is not affected.

With

(setq default-frame-alist
      '((background-mode  . dark)
	(background-color . "black")
	(foreground-color . "white")))

the tooltip text foreground is white, but the background is still the
default lightyellow, so the text is not visible.




[-- Attachment #3: Type: message/rfc822, Size: 3384 bytes --]

From: "Juanma Barranquero" <lekktu@gmail.com>
To: "martin rudalics" <rudalics@gmx.at>
Cc: 1032-done@emacsbugs.donarmstrong.com
Subject: Re: bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not
Date: Thu, 30 Oct 2008 02:43:27 +0100
Message-ID: <f7ccd24b0810291843l2edc56fbx3b275a029ba0ee81@mail.gmail.com>

Martin, I've fixed #1032 with a variant of the patch you sent.

The other issue, about tooltip-show modifying
tooltip-frame-parameters, seems deliberate. tooltip.el defines a
function tooltip-set-param with the explicit intent of modifying its
argument alist, and `tooltip-show' copies `tooltip-frame-parameters'
with copy-sequence, instead of copy-alist, and then uses
`tooltip-set-param' on it.

I can only imagine the idea was that `tooltip-frame-parameters' should
not contain misleading (fore|back)ground-color entries that weren't
really applied while creating the tooltip. Still, it seems weird to
modify under-the-hood a customizable variable.

             Juanma

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[martin rudalics]

 > The other issue, about tooltip-show modifying
 > tooltip-frame-parameters, seems deliberate. tooltip.el defines a
 > function tooltip-set-param with the explicit intent of modifying its
 > argument alist, and `tooltip-show' copies `tooltip-frame-parameters'
 > with copy-sequence, instead of copy-alist, and then uses
 > `tooltip-set-param' on it.

This doesn't make sense to me.  When `tooltip-show' calls `x-show-tip'
it does so with _params_ and not with `tooltip-frame-parameters'.
What's the reason to do the `copy-sequence' in the first place?  Maybe
it's all about the 'left or 'top paramters.

 > I can only imagine the idea was that `tooltip-frame-parameters' should
 > not contain misleading (fore|back)ground-color entries that weren't
 > really applied while creating the tooltip. Still, it seems weird to
 > modify under-the-hood a customizable variable.

At least, such a reset should occur at the time the variable is
customized and not when the tooltip is displayed.


BTW, a related issue: When I, for example, use the mouse for popping up

	"Tools->Version Control->Show History"
	
from the menubar I get the tooltip

     "List the change log of the current fileset in a window"

When I now move the mouse to the next menu entry

      "Tools->Version Control->Update ChangeLog"

the old tooltip remains displayed.  Do you see that?  Is that a w32-only
problem?

martin

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[Juanma Barranquero]

On Thu, Oct 30, 2008 at 08:24, martin rudalics <rudalics@gmx.at> wrote:

> This doesn't make sense to me.  When `tooltip-show' calls `x-show-tip'
> it does so with _params_ and not with `tooltip-frame-parameters'.
> What's the reason to do the `copy-sequence' in the first place?  Maybe
> it's all about the 'left or 'top paramters.

Judging from the code, I think the idea is not to modify
`tooltip-frame-parameters', *except* for
(foreground|background|border)-color, if they appear (if they don't,
they are not added). The reasoning being that allowing such parameters
with values that are not being obeyed would be misleading.

> At least, such a reset should occur at the time the variable is
> customized and not when the tooltip is displayed.

The right thing to do, IMHO, is to change tooltip-show not to modify
`tooltip-frame-parameters', and fix the variable's docstring to
explain that the color attributes of the tooltip frame come from the
tooltip face. WDYT about the attached patch?

> When I now move the mouse to the next menu entry
>
>     "Tools->Version Control->Update ChangeLog"
>
> the old tooltip remains displayed.  Do you see that?  Is that a w32-only
> problem?

I cannot reproduce it. The new tooltip appears after a small delay.

  Juanma


2008-10-30  Juanma Barranquero  <lekktu@gmail.com>

	* tooltip.el (tooltip-frame-parameters): Doc fix.
	(tooltip-set-param): Remove.
	(tooltip-show): Don't modify `tooltip-frame-parameters'.


Index: lisp/tooltip.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/tooltip.el,v
retrieving revision 1.89
diff -u -2 -r1.89 tooltip.el
--- lisp/tooltip.el	29 Oct 2008 12:10:33 -0000	1.89
+++ lisp/tooltip.el	30 Oct 2008 10:16:32 -0000
@@ -122,5 +122,8 @@

 If `left' or `top' parameters are included, they specify the absolute
-position to pop up the tooltip."
+position to pop up the tooltip.
+
+Note that font and color parameters are ignored, and the attributes
+of the `tooltip' face are used instead."
   :type 'sexp
   :group 'tooltip)
@@ -204,14 +207,4 @@
 ;;; Displaying tips

-(defun tooltip-set-param (alist key value)
-  "Change the value of KEY in alist ALIST to VALUE.
-If there's no association for KEY in ALIST, add one, otherwise
-change the existing association.  Value is the resulting alist."
-  (let ((param (assq key alist)))
-    (if (consp param)
-	(setcdr param value)
-      (push (cons key value) alist))
-    alist))
-
 (declare-function x-show-tip "xfns.c"
 		  (string &optional frame parms timeout dx dy))
@@ -233,12 +226,13 @@
       (tooltip-show-help-non-mode text)
     (condition-case error
-	(let ((params (copy-sequence tooltip-frame-parameters))
+	(let ((params tooltip-frame-parameters)
 	      (fg (face-attribute 'tooltip :foreground))
 	      (bg (face-attribute 'tooltip :background)))
 	  (when (stringp fg)
-	    (setq params (tooltip-set-param params 'foreground-color fg))
-	    (setq params (tooltip-set-param params 'border-color fg)))
+	    (setq params (append `((foreground-color . ,fg)
+				   (border-color . ,fg))
+				 params)))
 	  (when (stringp bg)
-	    (setq params (tooltip-set-param params 'background-color bg)))
+	    (setq params (cons (cons 'background-color bg) params)))
 	  (x-show-tip (propertize text 'face 'tooltip)
 		      (selected-frame)

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[martin rudalics]

> Judging from the code, I think the idea is not to modify
> `tooltip-frame-parameters', *except* for
> (foreground|background|border)-color, if they appear (if they don't,
> they are not added). The reasoning being that allowing such parameters
> with values that are not being obeyed would be misleading.

Now I understand.

> The right thing to do, IMHO, is to change tooltip-show not to modify
> `tooltip-frame-parameters', and fix the variable's docstring to
> explain that the color attributes of the tooltip frame come from the
> tooltip face. WDYT about the attached patch?

Looks good.  Did you check that parameters appearing later in the
list don't override parameters appearing earlier?

> I cannot reproduce it. The new tooltip appears after a small delay.

I see - it's 10 seconds here - not really small ;-)

martin

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[Juanma Barranquero]

On Thu, Oct 30, 2008 at 12:40, martin rudalics <rudalics@gmx.at> wrote:

> Looks good.

And, now that I think of it, the doc fix for
`tooltip-frame-parameters' should go in even if Stefan and/or Chong
oppose the rest of the patch.

> Did you check that parameters appearing later in the
> list don't override parameters appearing earlier?

I checked for color attributes and font, yes. But anyway, that's the
usual behavior of alists, unless the code is explicitly iterating over
them instead of using `assq' and friends...

> I see - it's 10 seconds here - not really small ;-)

Curious. It's more like half a second here.

  Juanma

bug#1032: foreground-color in default-frame-alist affects tooltips, while background-color does not

[martin rudalics]

 > Curious. It's more like half a second here.

It must be related to my .emacs.  With emacs -Q it's as you say.

martin