mouse-face highlighting broken?

mouse-face highlighting broken?

[David Reitter]

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

I'm trying to track down a problem with mouse faces, where some  
portions of text propertized with the a mouse-face do not respect the  
mouse face, or at least not the "background" attribute of the mouse  
face.  The types of properties I am concerned with here are the  
background of images and the space allocated using the display/ 
space/:width property.  (See also screenshot.)

It may be quicker for me to track down the problem in the code than to  
compose a standalone bug report.  Could someone tell me where such  
properties are evaluated?

Also, have there been any changes compared to 22.3 that could effect  
such a bug?
I tried undoing 9d77a3 (see below), but that wasn't it.



[-- Attachment #2: pastedGraphic.png --]
[-- Type: image/png, Size: 5466 bytes --]

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







2008-05-27  Chong Yidong  <cyd@stupidchicken.com>

	* xdisp.c (draw_glyphs): If mouse-highlighting is on, attempt to
	draw overlap glyphs with appropriate highlighting.

Re: mouse-face highlighting broken?

[Chong Yidong]

David Reitter <david.reitter@gmail.com> writes:

> I'm trying to track down a problem with mouse faces, where some
> portions of text propertized with the a mouse-face do not respect the
> mouse face, or at least not the "background" attribute of the mouse
> face.  The types of properties I am concerned with here are the
> background of images and the space allocated using the display/
> space/:width property.  (See also screenshot.)
>
> It may be quicker for me to track down the problem in the code than to
> compose a standalone bug report.  Could someone tell me where such
> properties are evaluated?

Maybe handle_display_prop is what you want.

Re: mouse-face highlighting broken?

[David Reitter]

On May 6, 2009, at 11:42 AM, Chong Yidong wrote:

> David Reitter <david.reitter@gmail.com> writes:
>
>> I'm trying to track down a problem with mouse faces, where some
>> portions of text propertized with the a mouse-face do not respect the
>> mouse face, or at least not the "background" attribute of the mouse
>> face.  The types of properties I am concerned with here are the
>> background of images and the space allocated using the display/
>> space/:width property.  (See also screenshot.)
>>
>> It may be quicker for me to track down the problem in the code than  
>> to
>> compose a standalone bug report.  Could someone tell me where such
>> properties are evaluated?
>
> Maybe handle_display_prop is what you want.

Thanks, but none of the few changes there cause my problem.

Can people on other systems try this:

(insert
  (propertize
   (concat "Hello"
	  (propertize " " 'display '(space :width 40))
	  "World")
   'mouse-face 'highlight
   ))

The highlight works ok in 22.  In 23, only "Hello" and "World" are  
highlighted - the space in between is not.
As said above, the same applies to backgrounds of images.

RE: mouse-face highlighting broken?

[Drew Adams]

> Can people on other systems try this:
> (insert
>   (propertize
>    (concat "Hello"
> 	  (propertize " " 'display '(space :width 40))
> 	  "World")
>    'mouse-face 'highlight))
> 
> The highlight works ok in 22.  In 23, only "Hello" and "World" are  
> highlighted - the space in between is not.
> As said above, the same applies to backgrounds of images.

In the pretest on Windows, the space between is also mouse-faced.

Re: mouse-face highlighting broken?

[David Reitter]

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

On May 6, 2009, at 1:51 PM, Drew Adams wrote:
>
> In the pretest on Windows, the space between is also mouse-faced.

Thanks Drew for this piece of info.
With that, I took a look at the relevant code in the NS port, and it  
turns out that ns_dumpglyphs_stretch() and ns_dumpglyphs_image() fail  
to use the mouse face instead of the first glyph face.  The patch  
below addresses that.

I suspect that ns_dumpglyphs_stretch() lacks support for stipple  
patterns, but I think we can skip over that for now.


diff --git a/src/nsterm.m b/src/nsterm.m
index 1b6478c..52b7a16 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2842,8 +2845,17 @@ ns_dumpglyphs_image (struct glyph_string *s,  
NSRect r)
    /* Draw BG: if we need larger area than image itself cleared, do  
that,
       otherwise, since we composite the image under NS (instead of  
mucking
       with its background color), we must clear just the image area. */
-  [ns_lookup_indexed_color (NS_FACE_BACKGROUND
-            (FACE_FROM_ID (s->f, s->first_glyph->face_id)), s->f) set];
+  if (s->hl == DRAW_MOUSE_FACE)
+    {
+      face = FACE_FROM_ID
+	(s->f, FRAME_NS_DISPLAY_INFO (s->f)->mouse_face_face_id);
+      if (!face)
+	face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
+    }
+  else
+    face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
+
+  [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f) set];

    if (bg_height > s->slice.height || s->img->hmargin || s->img- 
 >vmargin
        || s->img->mask || s->img->pixmap == 0 || s->width != s- 
 >background_width)
@@ -2914,6 +2926,7 @@ ns_dumpglyphs_stretch (struct glyph_string *s)
  {
    NSRect r[2];
    int n, i;
+  struct face *face;

    if (!s->background_filled_p)
      {
@@ -2954,9 +2967,21 @@ ns_dumpglyphs_stretch (struct glyph_string *s)
              }
          }

+
        ns_focus (s->f, r, n);
-      [ns_lookup_indexed_color (NS_FACE_BACKGROUND
-           (FACE_FROM_ID (s->f, s->first_glyph->face_id)), s->f) set];
+
+      if (s->hl == DRAW_MOUSE_FACE)
+	{
+	  face = FACE_FROM_ID
+	    (s->f, FRAME_NS_DISPLAY_INFO (s->f)->mouse_face_face_id);
+	  if (!face)
+	    face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
+	}
+      else
+	face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
+
+      [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f) set];
+
        NSRectFill (r[0]);
        NSRectFill (r[1]);
        ns_unfocus (s->f);



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: mouse-face highlighting broken?

[Chong Yidong]

David Reitter <david.reitter@gmail.com> writes:

> With that, I took a look at the relevant code in the NS port, and it
> turns out that ns_dumpglyphs_stretch() and ns_dumpglyphs_image() fail
> to use the mouse face instead of the first glyph face.  The patch
> below addresses that.

This patch looks reasonable to me.

Re: mouse-face highlighting broken?

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 07 May 2009 00:46:13 -0400, Chong Yidong <cyd@stupidchicken.com> said:

>> With that, I took a look at the relevant code in the NS port, and
>> it turns out that ns_dumpglyphs_stretch() and ns_dumpglyphs_image()
>> fail to use the mouse face instead of the first glyph face.  The
>> patch below addresses that.

> This patch looks reasonable to me.

It might be OK to apply such a change for now, but I'd rather think if
the NS port had a similar code structure to the other terms, an
oversight like this case would hardly happen in the first place.  The
other terms consolidate the mouse face handling into one function
x_set_mouse_face_gc rather than scattering it into 3 (the proposed
change makes it 5) places.  Also, this uniformity among different
terms makes it easier for other terms to catch up with xterm.c
changes.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: mouse-face highlighting broken?

[David Reitter]

On May 7, 2009, at 2:29 AM, YAMAMOTO Mitsuharu wrote:
> The
> other terms consolidate the mouse face handling into one function
> x_set_mouse_face_gc rather than scattering it into 3 (the proposed
> change makes it 5) places.  Also, this uniformity among different
> terms makes it easier for other terms to catch up with xterm.c
> changes.

Yes, some refactoring is definitely in order after the release.
Cleaner code would be nice, things like ns_tmp_flags are just confusing.

Also, better comments would make things much easier, particularly in  
xdisp.c. 

Re: mouse-face highlighting broken?

[Stefan Monnier]

> Also, better comments would make things much easier, particularly in
> xdisp.c. 

Comment improvements can be installed at any time, so please send them
along and/or install them yourself.


        Stefan