561388 - unexpected horizontal line repaint issue in pull-down menulist

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Alice0775 White]

Attachment: screenshot

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100423
Minefield/3.7a5pre ID:20100423040128.
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100423
Minefield/3.7a5pre ID:20100423040128.

Option > Applications pane
When i mouse over pull-down list of actions Menu, unexpected horizontal line
will remain.
It happens with "Windows Classic" visual style. does not happens with "Aero Peek" visual style.

A similar Bug 549630, but it is different regression range.

Reproducible: Always

Steps to Reproduce:
1.Start Minefield with New profile
2.Option > Applications pane
3.Mouse over pull-down list of actions Menu

Actual Results:  
unexpected horizontal line will remain


Expected Results:  
No unexpected horizontal line

regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/560eb81389cc
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100422
Minefield/3.7a5pre ID:20100422172405

Fails:
http://hg.mozilla.org/mozilla-central/rev/a9ec36359a4c
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100422
Minefield/3.7a5pre ID:20100422211905

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=560eb81389cc&tochange=a9ec36359a4c

Comment 1

[Alice0775 White]

Checked in Bug 556052 cause this probrem.

Comment 2

[Alfred Kayser]

The problem also happens in Thunderbird nightlies since about 25-4-2010.

Comment 3

[Daniel Holbert [:dholbert]]

Alice: Your 'fails' & 'works' build IDs are identical in comment 0 -- I think you meant the 'fails' one to be a day later, for 4/23. Assuming that's correct, I can confirm this bug on Linux, with the same nightly regression range.  Seems to happen specifically when a <select> box is at a fractional y-position.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

This testcase reproduces the issue for me, starting with the 2010-04-23 nightly.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: fix?

Was looking at this code for zoom stuff and I think I stumbled upon the problem here. We add the ViewToWidgetOffset to view coordinates to get widget coordinates, so we should subtract the widget coordinates from the view coordinates when calculating this offset.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See bug 556052 comment 27. This patch regresses bug 556052. Especially see this testcase on Mac. You probably don't have a Mac though...
https://bugzilla.mozilla.org/attachment.cgi?id=442538

Having said that, I think your patch is right, and I was wrong in bug 556052 comment 27.

Comment 8

[Alfred Kayser]

Comment on attachment 442446 [details] [diff] [review]
fix?

Can we get this patch through the process of reviewing, approving and submitting?

Comment 9

[Timothy Nikkel (:tnikkel)]

Comment on attachment 442446 [details] [diff] [review]
fix?

This patch causes other regressions, so we can't take it until we fix those other regressions.

Comment 10

[(not currently active) Ted Mielczarek]

Hasn't this bug cropped up before? Do we not have a test for this?

Comment 11

[(not currently active) Ted Mielczarek]

Bleh, bug 472769 was the previous issue, and it does have reftests. I guess they just don't cover this case!

Comment 12

[Timothy Nikkel (:tnikkel)]

I think the reason that we do not have a test for this is that there is currently no way to test this.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: testcase

mViewToWidgetOffset is confusing me again.

In this testcase the frame tree looks like this:
Viewport(-1)@0x14f28b0 [view=0x19075eb0] {0,0,75120,51360} [state=00042020] [content=0x0] [sc=0x14f2730] pst=:-moz-viewport<
  HTMLScroll(html)(-1)@0x223912d8 {0,0,75120,51360} [state=00040010] [content=0x1907f380] [sc=0x22390e10] pst=:-moz-viewport-scroll<
    Canvas(html)(-1)@0x22391158 {0,0,75120,51360} [state=00040000] [content=0x1907f380] [sc=0x22391030] pst=:-moz-scrolled-canvas<
      Block(html)(-1)@0x22397218 [content=0x1907f380] {0,0,75120,480} [state=00d00000] sc=0x223971b8(i=0,b=1)<
        line 0x22397610: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8348] bm=480 {480,0,74160,0} ca={480,480,74160,0} <
          Block(body)(1)@0x223975a8 [content=0x19081040] {480,480,74160,0} [state=00100000] sc=0x223974a8(i=1,b=0)<
            line 0x223978c8: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8300] {0,0,0,0} <
              Placeholder(embed)(1)@0x22397890 {0,0,0,0} [state=00200000] [content=0x19084550] outOfFlowFrame=ObjectFrame(embed)(1)@0x22397800
            >
          >
        >
      >
    >
    Absolute-list<
      ObjectFrame(embed)(1)@0x22397800 [view=0x19084920] {7218,600,6000,6000} [state=00002110] [content=0x19084550]
    >
  >
>

In nsIView::CalcWidgetBounds, newBounds.x is 120, which is correct (the edge is snapped from 120.3 to the nearest pixel). But since the view origin is at 120.3, the view-to-widget offset should be 0.3px. But with your patch it's -0.3.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #5)
> Was looking at this code for zoom stuff and I think I stumbled upon the problem
> here. We add the ViewToWidgetOffset to view coordinates to get widget
> coordinates, so we should subtract the widget coordinates from the view
> coordinates when calculating this offset.

The thing is, the current code *does* subtract widget coordinates. roundedOffset.TopLeft() is the top-left of the widget, relative to the parent widget.

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to comment #15)
> The thing is, the current code *does* subtract widget coordinates.
> roundedOffset.TopLeft() is the top-left of the widget, relative to the parent
> widget.

Yeah, looks like the patch is wrong.

Comment 18

[Timothy Nikkel (:tnikkel)]

Bug 575146 comment 3 indicates D2D is affected differently by this bug. Or at least I think it is the same bug. This is a reminder comment to check that D2D is also fixed when this bug is fixed.

Comment 20

[Jim Mathies [:jimm]]

The new client area patches may have touched this and made it more common. Or maybe everybody is just hyper sensitive to drawing related issues right now. :)

Comment 21

[Timothy Nikkel (:tnikkel)]

I can't reproduce on the testcase or in the places I usually see this. Does anybody else still see this? I wonder if retained layers fixed this.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

I used to see it in my own builds on some sites, but I can't reproduce anymore either.

Comment 23

[Cameron McCormack (:heycam)]

With nightlies I no longer see the problem I reported in bug 575146.

Comment 24

[Timothy Nikkel (:tnikkel)]

Calling this fixed. If anyone still sees the problem please reopen.

Comment 25

[Stephen Donner [:stephend] Not actively reading bugmail]

I used to see this on Cars.com w/trunk, but no longer there, either; thanks!

Comment 26

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

This was probably fixed when patch for bug #579264 landed

Comment 27

[Alfred Kayser]

Marking as fixed by bug 579264.
I verified this with an old build of Shredder and my theme Nautipolis which had serious 'horizontal lines' issues, but with the current nightly of Shredder the lines don't appear anymore.

Comment 28

[Timothy Nikkel (:tnikkel)]

For bug archaeology purposes, it was not bug 579264. The fix range was the retained layers landing, not bug 579264.