unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame]
@ 2007-02-03 11:19 Richard Stallman
  2007-02-04  0:35 ` Chong Yidong
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Stallman @ 2007-02-03 11:19 UTC (permalink / raw)
  To: emacs-devel

Would someone please investigate this and DTRT?

------- Start of forwarded message -------
To: emacs-pretest-bug@gnu.org
From: Leo <sdl.web@gmail.com>
Date: Fri, 02 Feb 2007 14:26:31 +0000
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: scroll-bar face gets changed in a new frame
X-Spam-Status: No, score=0.0 required=5.0 tests=none autolearn=failed 
	version=3.0.4

On 2007-01-07, Leo said:

> Hi all,
>
> I suspect this is a bug. Tested in GNU Emacs 22.0.92.2
> (i686-pc-linux-gnu, GTK+ Version 2.6.4) of 2007-01-07 on soup
>
> Start Emacs with "emacs -q -l emacs-custom".
>
> where emacs-custom is this:
>
> ,----[ emacs-custom ]
> | (custom-set-faces
> |   ;; custom-set-faces was added by Custom.
> |   ;; If you edit it by hand, you could mess it up, so be careful.
> |   ;; Your init file should contain only one such instance.
> |   ;; If there is more than one, they won't work right.
> |  '(scroll-bar ((t (:background "#2e3436" :foreground "#eeeeec")))))
> `----
>
> In the initial frame:
>
> ,----[ M-x describe-face RET scroll-bar ]
> |      ...
> |      Foreground: #eeeeec
> |      Background: #2e3436
> |      ...
> `----
>
> Then make a new frame with 'C-x 5 2' and it becomes
>
> ,----[ M-x describe-face RET scroll-bar ]
> |      ...
> |      Foreground: #101010
> |      Background: #fbf8f1
> |      ...
> `----
>
> In emacs that compiles with "--without-toolkit-scroll-bars", you can
> see the change visually.

This bug is still in GNU Emacs 22.0.93.3 (i686-pc-linux-gnu, GTK+
Version 2.10.8) of 2007-02-02

- -- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)



_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame]
  2007-02-03 11:19 [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame] Richard Stallman
@ 2007-02-04  0:35 ` Chong Yidong
  2007-02-04  1:25   ` Leo
  2007-02-06 22:38   ` Chong Yidong
  0 siblings, 2 replies; 5+ messages in thread
From: Chong Yidong @ 2007-02-04  0:35 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Would someone please investigate this and DTRT?

The problem is in face-set-after-frame-default.  The function merges
in the X resources, then initializes attributes from frame parameters.
However, if there is a scroll-bar X resource present, the call to
make-face-x-resource-internal will chang the scroll-bar-* frame
parameters in the process of changing the scroll-bar face.  Therefore,
any previously-applied frame parameters are lost.

I think one fix is for face-set-after-frame-default to first construct
a list of frame parameters to be applied (based on the new frame's
frame-parameters and/or the face's value for new frames), then wait
until after the call to make-face-x-resource-internal before applying
these changes.  See the attached patch.

By the way, the symbol inhibit-default-face-x-resources seems to be a
no-op; it is called as

 (when (not (equal face 'default))
   ...
   (when (and (memq window-system '(x w32 mac))
     (or (not (boundp 'inhibit-default-face-x-resources))
         (not (eq face 'default))))
     (make-face-x-resource-internal face frame)))

However, since face is never `default' inside the `when' form, the
enclosing `or' statement is always t, regardless of whether or not
inhibit-default-face-x-resources is bound.  I think this symbol was
introduced to solve a similar problem, and it can be removed if the
proposed fix is applied.

*** emacs/lisp/faces.el.~1.362.~	2007-02-02 11:55:30.000000000 -0500
--- emacs/lisp/faces.el	2007-02-03 19:28:18.000000000 -0500
***************
*** 1754,1788 ****
  			  (face-attribute 'default :weight t))
        (set-face-attribute 'default frame :width
  			  (face-attribute 'default :width t))))
!   (dolist (face (face-list))
!     ;; Don't let frame creation fail because of an invalid face spec.
!     (condition-case ()
! 	(when (not (equal face 'default))
! 	  (face-spec-set face (face-user-default-spec face) frame)
! 	  (internal-merge-in-global-face face frame)
! 	  (when (and (memq window-system '(x w32 mac))
! 		     (or (not (boundp 'inhibit-default-face-x-resources))
! 			 (not (eq face 'default))))
! 	    (make-face-x-resource-internal face frame)))
!       (error nil)))
!   ;; Initialize attributes from frame parameters.
    (let ((params '((foreground-color default :foreground)
  		  (background-color default :background)
  		  (border-color border :background)
  		  (cursor-color cursor :background)
  		  (scroll-bar-foreground scroll-bar :foreground)
  		  (scroll-bar-background scroll-bar :background)
! 		  (mouse-color mouse :background))))
      (dolist (param params)
!       (let ((frame-param (frame-parameter frame (nth 0 param)))
! 	    (face (nth 1 param))
! 	    (attr (nth 2 param)))
! 	(when (and frame-param
! 		   ;; Don't override face attributes explicitly
! 		   ;; specified for new frames.
! 		   (eq (face-attribute face attr t) 'unspecified))
! 	  (set-face-attribute face frame attr frame-param))))))
! 
  
  (defun tty-handle-reverse-video (frame parameters)
    "Handle the reverse-video frame parameter for terminal frames."
--- 1754,1790 ----
  			  (face-attribute 'default :weight t))
        (set-face-attribute 'default frame :width
  			  (face-attribute 'default :width t))))
!   ;; Find attributes that should be initialized from frame parameters.
    (let ((params '((foreground-color default :foreground)
  		  (background-color default :background)
  		  (border-color border :background)
  		  (cursor-color cursor :background)
  		  (scroll-bar-foreground scroll-bar :foreground)
  		  (scroll-bar-background scroll-bar :background)
! 		  (mouse-color mouse :background)))
! 	apply-params)
      (dolist (param params)
!       (let* ((value (frame-parameter frame (nth 0 param)))
! 	     (face (nth 1 param))
! 	     (attr (nth 2 param))
! 	     (default (face-attribute face attr t)))
! 	;; Don't set the attributes yet, since the call to
! 	;; make-face-x-resource-internal may change frame parameters.
! 	(if (eq default 'unspecified)
! 	    (if value
! 		(push (list face frame attr value) apply-params))
! 	  (push (list face frame attr default) apply-params))))
!     (dolist (face (delq 'default (face-list)))
!       ;; Don't let frame creation fail because of an invalid face spec.
!       (condition-case ()
! 	  (progn
! 	    (face-spec-set face (face-user-default-spec face) frame)
! 	    (internal-merge-in-global-face face frame)
! 	    (when (memq window-system '(x w32 mac))
! 	      (make-face-x-resource-internal face frame)))
! 	(error nil)))
!     (dolist (param apply-params)
!       (apply 'set-face-attribute param))))
  
  (defun tty-handle-reverse-video (frame parameters)
    "Handle the reverse-video frame parameter for terminal frames."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame]
  2007-02-04  0:35 ` Chong Yidong
@ 2007-02-04  1:25   ` Leo
  2007-02-06 22:38   ` Chong Yidong
  1 sibling, 0 replies; 5+ messages in thread
From: Leo @ 2007-02-04  1:25 UTC (permalink / raw)
  To: emacs-devel

On 2007-02-04, Chong Yidong said:

> Richard Stallman <rms@gnu.org> writes:
>
>> Would someone please investigate this and DTRT?
>
> The problem is in face-set-after-frame-default.  The function merges
> in the X resources, then initializes attributes from frame parameters.
> However, if there is a scroll-bar X resource present, the call to
> make-face-x-resource-internal will chang the scroll-bar-* frame
> parameters in the process of changing the scroll-bar face.  Therefore,
> any previously-applied frame parameters are lost.
>
> I think one fix is for face-set-after-frame-default to first construct
> a list of frame parameters to be applied (based on the new frame's
> frame-parameters and/or the face's value for new frames), then wait
> until after the call to make-face-x-resource-internal before applying
> these changes.  See the attached patch.
>
> By the way, the symbol inhibit-default-face-x-resources seems to be a
> no-op; it is called as
>
>  (when (not (equal face 'default))
>    ...
>    (when (and (memq window-system '(x w32 mac))
>      (or (not (boundp 'inhibit-default-face-x-resources))
>          (not (eq face 'default))))
>      (make-face-x-resource-internal face frame)))
>
> However, since face is never `default' inside the `when' form, the
> enclosing `or' statement is always t, regardless of whether or not
> inhibit-default-face-x-resources is bound.  I think this symbol was
> introduced to solve a similar problem, and it can be removed if the
> proposed fix is applied.
>
[patch]

I confirm it fixes the bug.

-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame]
  2007-02-04  0:35 ` Chong Yidong
  2007-02-04  1:25   ` Leo
@ 2007-02-06 22:38   ` Chong Yidong
  2007-02-11  2:56     ` Leo
  1 sibling, 1 reply; 5+ messages in thread
From: Chong Yidong @ 2007-02-06 22:38 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> The problem is in face-set-after-frame-default.  The function merges
> in the X resources, then initializes attributes from frame parameters.
> However, if there is a scroll-bar X resource present, the call to
> make-face-x-resource-internal will chang the scroll-bar-* frame
> parameters in the process of changing the scroll-bar face.  Therefore,
> any previously-applied frame parameters are lost.
>
> I think one fix is for face-set-after-frame-default to first construct
> a list of frame parameters to be applied (based on the new frame's
> frame-parameters and/or the face's value for new frames), then wait
> until after the call to make-face-x-resource-internal before applying
> these changes.

I checked in the patch.  It should be safe, but please keep an eye out
for any unexpected interaction between X resources and frame
parameters.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame]
  2007-02-06 22:38   ` Chong Yidong
@ 2007-02-11  2:56     ` Leo
  0 siblings, 0 replies; 5+ messages in thread
From: Leo @ 2007-02-11  2:56 UTC (permalink / raw)
  To: emacs-devel

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

Hello, Chong!

On 2007-02-06, Chong Yidong said:

> Chong Yidong <cyd@stupidchicken.com> writes:
>
>> The problem is in face-set-after-frame-default.  The function
>> merges in the X resources, then initializes attributes from frame
>> parameters.  However, if there is a scroll-bar X resource present,
>> the call to make-face-x-resource-internal will chang the
>> scroll-bar-* frame parameters in the process of changing the
>> scroll-bar face.  Therefore, any previously-applied frame
>> parameters are lost.
>>
>> I think one fix is for face-set-after-frame-default to first
>> construct a list of frame parameters to be applied (based on the
>> new frame's frame-parameters and/or the face's value for new
>> frames), then wait until after the call to
>> make-face-x-resource-internal before applying these changes.
>
> I checked in the patch.  It should be safe, but please keep an eye
> out for any unexpected interaction between X resources and frame
> parameters.

Mode-line face has a similar bug.

To reproduce:
   1) emacs -Q -l ml
   2) C-x 5 2


[-- Attachment #2: ml --]
[-- Type: text/plain, Size: 366 bytes --]

;;-*-Emacs-Lisp-*-
(custom-set-faces
  ;; custom-set-faces was added by Custom.
  ;; If you edit it by hand, you could mess it up, so be careful.
  ;; Your init file should contain only one such instance.
  ;; If there is more than one, they won't work right.
 '(mode-line ((t (:background "#2e3436" :foreground "#eeeeec" :box (:line-width -1 :color "#555753"))))))

[-- Attachment #3: Type: text/plain, Size: 84 bytes --]

regards,
-- 
Leo <sdl.web AT gmail.com>                         (GPG Key: 9283AA3F)

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-02-11  2:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-02-03 11:19 [sdl.web@gmail.com: Re: scroll-bar face gets changed in a new frame] Richard Stallman
2007-02-04  0:35 ` Chong Yidong
2007-02-04  1:25   ` Leo
2007-02-06 22:38   ` Chong Yidong
2007-02-11  2:56     ` Leo

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).