424465 - [FIX]CSS border interruption when setting innerHTML of children of form that has margin-bottom set and no horizontal resize needed.

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P2)

Description

[Raif Atef Wasef]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre

Check the testcase, it actually shows 2 issues.

First of all, I establish a session and send the form.
Choose something from the drop down lists and press Submit.
A javascript function causes an image that was originally loaded with display:none to be displayed.

When the image loads, the border is slightly cut off. If you open another window anywhere that covers that part of the page, or right-click a context menu item in the page that covers the affected problem and close it, it redraws correctly.

2nd Issue: Press F5 to reload page, both drop down boxes are still disabled. CTRL+F5 fixes the issue. They were disabled by me (js) when you press Submit.

This is strange because I explicitly send no cache headers to prevent fx from reloading from the cache. Fx2 does not exhibit this problem and neither does IE5.5+

Reproducible: Always

Steps to Reproduce:
see details...
Actual Results:  
1. Border interruption, redraws correctly when region is covered by another window or context menu.
2. Disabled state of dropdown boxes not cleared on normal reload.

Expected Results:  
1. Border displayed properly.
2. Dropdown box disabled state reset to normal (selectable).

Running on a Dell Inspiron 4100, P3 866 MHz, ATI Radeon Mobility M6 Card, Unofficial ATi omega display driver version 3.8.252

Comment 1

[Raif Atef Wasef]

Attachment: Screenshot of 1st problem

Comment 2

[Mats Palmgren (inactive)]

Can't connect to the site.

Comment 3

[Raif Atef Wasef]

I moved the testcase to another location, please see...

Comment 4

[Raif Atef Wasef]

Attachment: Minimized testcase directly showing the problem

I've been minimizing and playing with this a lot and discovered that it only happens if margin-bottom is set on the form enclosing it.

I changed from php headers to meta tags and attached here for easy viewing.

Comment 5

[Steve England [:stevee]]

Rounding bug in the invalidation code?

Comment 6

[Raif Atef Wasef]

Unfortunately I can't say, because I am still a C/C++ newbie and I'm not familiar with Mozilla XUL or source code.

Comment 7

[Raif Atef Wasef]

I split the 2nd problem into a separate bug: https://bugzilla.mozilla.org/show_bug.cgi?id=425083

Sorry for all the bug spam, I am new to bugzilla (and bug reporting in general).

Comment 8

[Dão Gottwald [:dao]]

needs a regression range

Comment 9

[Steve England [:stevee]]

Attachment: smaller testcase that shows a bigger problem

Comment 10

[Steve England [:stevee]]

Works: 20080206_1337_firefox-3.0b4pre.en-US.win32
Broken: 20080206_1337_firefox-3.0b4pre.en-US.win32

Checkins to module PhoenixTinderbox between 2008-02-06 13:37 and 2008-02-06 14:32 :
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-06+13%3A37&maxdate=2008-02-06+14%3A32&cvsroot=%2Fcvsroot

Due to bug 414298 I'd say. CC'ng bz and requesting blocking.

Comment 11

[Steve England [:stevee]]

of course, should be
Broken: 20080206_1432_firefox-3.0b4pre.en-US.win32

Comment 12

[Steve England [:stevee]]

Attachment: shows regression in rendering

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Fun times.  When reflow branch landed, it removed all the InvalidateDamage() callers.  So after that, if a caption or the inner table got resized and the resize area included stuff that's not covered by any kids (the cellspacing in this case), we wouldn't repaint correctly.

Bernd, can you check to make sure it's OK to not actually use InvalidateDamage() here?  I think it should be fine, but...

Comment 14

[Bernd]

Comment on attachment 311721 [details] [diff] [review]
Proposed fix

Looks like my handling of bug 219141 was not correct. Given the history of bug 414298 I would say its time to put some reftests into the tree. At least for this bug the r+ requires reftests to be checked in with the patch.

Comment 15

[Boris Zbarsky [:bzbarsky]]

I would love to do that, but reftests can't catch invalidation bugs, since they do a fresh toplevel paint.  We need an entirely new test infrastructure, with backend support, to test this sort of thing.  :(  roc is planning to make Compositor support it, but that's not going to help me write tests for this patch...

Comment 16

[Bernd]

I just got very nervous when I looked at the regression chain. So no reftest for now. Indeed hovering with a window makes a new paint and then everything is OK. 

Comment 17

[Boris Zbarsky [:bzbarsky]]

Yeah, nervous is right.  :(

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 311721 [details] [diff] [review]
Proposed fix

Requesting approval.  This makes sure to invalidate correctly when the inner table or caption size changes.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 21

[Carsten Book [:Tomcat]]

verified fixed using the testcases and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041217 Minefield/3.0pre ID:2008041217

--> Verified fixed

Comment 22

[Jonathan Griffin (:jgriffin)]

Attachment: invalidate reftest

Comment 23

[Boris Zbarsky [:bzbarsky]]

Pushed test as http://hg.mozilla.org/mozilla-central/rev/4cbb996fbb60