374765 - selection colors are sometimes wrong

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: screenshot #1

Sometimes, maybe if an image is existing and there are text around it,  the selection text colors are wrong. I think that this is a regression of cairo build. But I don't know the regression point.

See the screenshots.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: screenshot #2

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: screenshot #3

after invalidate, it is drawn normally.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I tested with save/restore methods that is same as
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/generic&command=DIFF_FRAMESET&rev1=3.629&rev2=3.630

but this bug is still reproduced...

Comment 4

[-fullmetaljacket-]

(In reply to comment #0)
> Sometimes, maybe if an image is existing and there are text around it,  the
> selection text colors are wrong. 

i also see this even on selection without image (e.g. bug is also present on bugzilla search result)

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase #1

STEPS TO REPRODUCE
1. left-click-and-hold in the middle of the first paragraph
2. drag up and down a few times (selecting/deselecting the text)

ACTUAL RESULT
The text in the last line of the first paragraph sometimes have a
light-blue foreground color (it should be white on blue, on Linux).
Occasionally there is a light-blue stripe between the paragraphs.

EXPECTED RESULT
All selected text should have the same foreground/background color.
No light-blue stripe between the paragraphs.

The bug occurs in Firefox nightly 20070321 on Linux, but not in a
SeaMonkey nightly build of the same date.

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

This fixes it for me on Linux, I haven't tested it on Windows...

Comment 7

[-fullmetaljacket-]

testcase #1 is WFM using

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070321 Minefield/3.0a3pre ID:2007032115 [cairo]

Comment 8

[Mats Palmgren (inactive)]

The patch also fixes the URL/testcase on Windows XP for me.

Comment 9

[Mats Palmgren (inactive)]

Comment on attachment 259285 [details] [diff] [review]
Patch rev. 1

The problem seems to be a missing "ctx->NewPath()" in
nsDisplaySelectionOverlay::Paint
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.713&root=/cvsroot&mark=926,965-966,971-972#926

However, there is a nsIRenderingContext::FillRect() that does what we want:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/thebes/nsThebesRenderingContext.cpp&rev=1.42&root=/cvsroot&mark=666,672,702-704#665

It also seems to have a workaround for a cairo bug and it has logging...
so unless there is a specific reason to use thebes context directly
I suggest we use FillRect() instead.

Comment 10

[Stuart Parmenter]

Comment on attachment 259285 [details] [diff] [review]
Patch rev. 1

we're getting rid of nsIRenderingContext so it would be good not to add more code using it.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

Ok, here's the missing line then.

Comment 12

[Mats Palmgren (inactive)]

Checked in to trunk at 2007-03-24 02:21 PST.

-> FIXED

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> v.

Thank you, Mats!