124507 - Border does not display when hidden iframe becomes visible

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Kirk Parsons]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8) Gecko/20020204
BuildID:    2002020406

A hidden IFrame doesn't display it's border, until after you've done a refresh.

Reproducible: Always
Steps to Reproduce:
1.Load the attached "ns6 refresh bug 1.htm" page and notice that the border of
the iFrame is not visible. 
2. Resize the window, and notice the iFrame border becomes visible.


Actual Results:  1.Load the attached "ns6 refresh bug 1.htm" page and notice
that the border of the iFrame is not visible. 
2. Resize the window, and notice the iFrame border becomes visible.

Expected Results:  1.Load the attached "ns6 refresh bug 1.htm" page and the
iFrame border should be immediately visible.

Comment 1

[Kirk Parsons]

Attachment: htm file with iFrame that exhibits this bug.

Comment 2

[Kirk Parsons]

Attachment: simple content for iFrame

Comment 3

[Kirk Parsons]

Attachment: Corrected iFrame test exhibiting bug

Comment 4

[Kirk Parsons]

Attachment: Corrected iFrame test exhibiting bug

Hopefully I got it right, this time.

Comment 5

[Kirk Parsons]

Attachment: Corrected iFrame test exhibiting bug

Try again :(

Comment 6

[Sivakiran Tummala]

The border becomes visible after resizing or selecting the text in the iframe.
Setting proirity to P4.

Comment 7

[Johnny Stenback (:jst)]

Not a DOM problem, over to jkeiser since he's working on frame/iframe issues now.

Comment 8

[Kirk Parsons]

I think this should be marked as higher than P4.  Any web site/app which
alternately hides/shows IFRAMEs will encounter this bug frequently, and with
substantial negative effect.  It really makes IFRAMEs look down right lousy in
Mozilla, if you're doing DHTML apps.

Comment 9

[Jaime Rodriguez, Jr.]

Nominating for nsbeta1 and topembed. Changing to major/ADT2 because this causes
isseus for DHMTL developers wanting to hide iFrames. Dialogs do not look good
without borders, in the context of rendering a fundamental element of the
browser. this impacts the way DHTML developers are going to regard MachV. 

Comment 10

[Kevin McCluskey (gone)]

Looks like an invalidation problem. Dragging another window over the browser
window causes the border to be displayed.

Comment 11

[Kevin McCluskey (gone)]

nsbeta1+

Comment 12

[John Keiser (jkeiser)]

Attachment: Patch

This patch fixes the bug.  The problem is the frame itself is not being
invalidated, only the DocShell underneath.  If a frame's Reflow method is
responsible for invalidating itself, as I have been told, this is the correct
fix.  However, I am not convinced that this is the case, as I haven't found any
reflow methods that *do* invalidate the frame.	Also it makes more sense to me
that the caller invalidate the frame, as it can generate fewer invalidate calls
to invalidate a wider area encompassing all children.

But if I can't get an answer on that one soon, this patch should do ya good
enough for 1.0 (wallpaper you want, wallpaper you get).

Comment 13

[John Keiser (jkeiser)]

Attachment: Patch v2.0

This patch is better, more surgical.  The other one would have caused
unnecessary paints.

This one pulls the invalidation logic out of nsBlockFrame and puts it in its
own function in nsHTMLContainer (the nearest HTML-specific common ancestor),
which nsFrameOuterFrame then uses.  The logic is unaltered but it looks
somewhat different structurally because I had to determine if this was a style
change within the function instead of using a PRBool that was being set in the
function.

nsFieldSetFrame needs to do this as well (it has the same sort of error), but I
am saving that for now because having this bug in there causes other fieldset
invalidation bugs to be exposed.  bug 65566 and bug 103128 are likely
candidates to be fixed by it.

Comment 14

[Chris Waterson]

InvalidateBorder doesn't need to return an nsresult, don't add COM baloney where
it's not needed. :-)

Other than that, r=waterson.

Comment 15

[Marc Attinasi]

Comment on attachment 81214 [details] [diff] [review]
Patch v2.0

I noticed this, which you will want to address:

\ No newline at end of file

Also, should InvalidateBorder be called MaybeInvalidateBorder, or
InvalideBorderIfRequired?

sr=attinasi

Comment 16

[John Keiser (jkeiser)]

Per email exchange with Marc, calling it CheckInvalidateBorder().

Comment 17

[John Keiser (jkeiser)]

Fix checked in last night.

Comment 18

[chris hofmann]

Comment on attachment 81214 [details] [diff] [review]
Patch v2.0

a=chofmann for the 1.0 branch

Comment 19

[scottputterman]

adding adt1.0.0-.  Let's get this for rtm.

Comment 20

[Jaime Rodriguez, Jr.]

This may need a refresh a= from Drivers as this was approved by them on 04/28.
Renominating for adt1.0.0 for the 1.0 branch.

Comment 21

[scottputterman]

Let's get this verified on the trunk before considering it for checkin to the
branch.

Comment 22

[Kevin McCluskey (gone)]

[e-mail]: Can you verify this on the trunk? thanks.

Comment 23

[scottputterman]

stummala, could you take a look at this on the trunk?

Comment 24

[Prashant Desale]

I'm covering DOM area for stummala who is on medical leave. Sorry it took me
sometime to get to this bug, since bugs under stummala were under process of
assignment. 

I tested this one 2002-052108-trunk & everything seems working fine.
Border does display when hidden iframe becomes visible.
Marking this one verified.

Comment 25

[scottputterman]

adding adt1.0.0+.  Please get drivers approval again since it's been more than 3
days and then check into the 1.0 branch.

Comment 26

[scottputterman]

changing to adt1.0.1+ for checkin to the 1.0 branch.  Please get drivers
approval before checking in.

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

re-approved for 1.0.1
please check into the 1.0.1 branch ASAP. once landed remove the 
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword

Comment 28

[John Keiser (jkeiser)]

checked in to branch 5/31.

I just noticed something on trunk though, I committed some debug code that
causes us to invalidate more :(  Patch forthcoming.

Comment 29

[John Keiser (jkeiser)]

Attachment: Trunk Correction Patch

This removes the extra debug code I committed to trunk.  It is unnecessary
(though harmless) and is already handled by CheckInvalidateBorder().

Comment 30

[Bernd]

Comment on attachment 85971 [details] [diff] [review]
Trunk Correction Patch

r=bernd

Comment 31

[Sivakiran Tummala]

verified on win32 and linux 07-10-07-1.0 branch