Last Comment Bug 312942 - [FIX]two frameset tags cause page to appear blank
: [FIX]two frameset tags cause page to appear blank
Status: VERIFIED FIXED
: fixed1.8, regression
Product: Core
Classification: Components
Component: Layout: HTML Frames (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8rc1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
http://www.tvgenial.com/
Depends on:
Blocks: 309873 312097
  Show dependency treegraph
 
Reported: 2005-10-18 17:59 PDT by Frank Yan (:fryn)
Modified: 2011-08-05 21:29 PDT (History)
4 users (show)
asa: blocking1.8rc1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Broken Frameset Test Case (468 bytes, text/html)
2005-10-18 18:46 PDT, Frank Yan (:fryn)
no flags Details
Working Frameset Test Case (394 bytes, text/html)
2005-10-18 18:48 PDT, Frank Yan (:fryn)
no flags Details
Patch (1.98 KB, patch)
2005-10-18 19:44 PDT, Boris Zbarsky [:bz] (still a bit busy)
bzbarsky: review-
bzbarsky: superreview-
Details | Diff | Splinter Review
Better patch (4.37 KB, patch)
2005-10-18 21:59 PDT, Boris Zbarsky [:bz] (still a bit busy)
jst: review+
jst: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Frank Yan (:fryn) 2005-10-18 17:59:29 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051018 Firefox/1.4.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051018 Firefox/1.4.1

if a web page include two frameset tags, the page will appear as a blank white page.

Reproducible: Always
Comment 1 Frank Yan (:fryn) 2005-10-18 18:07:04 PDT
Boris, could you take a look at this bug, and see what component this bug
belongs to, and maybe create a patch?

Thanks.
Comment 2 Frank Yan (:fryn) 2005-10-18 18:08:23 PDT
Asa, I know you're busy, but could you refer this bug to someone who could
confirm this and maybe fix it if valid?

Thanks.
Comment 3 Josh Birnbaum 2005-10-18 18:35:18 PDT
The DOM looks correct, so this is probably the correct component.  I see the
problem with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b5)
Gecko/20051018 Firefox/1.4.1.

Marking regression because 1.7.12 works ok on the same machine.

Studio17: attaching a reduced testcase would be a good next step.
Comment 4 Frank Yan (:fryn) 2005-10-18 18:46:18 PDT
Created attachment 200027 [details]
Broken Frameset Test Case
Comment 5 Frank Yan (:fryn) 2005-10-18 18:48:38 PDT
Created attachment 200028 [details]
Working Frameset Test Case
Comment 6 Frank Yan (:fryn) 2005-10-18 18:52:26 PDT
i've made two testcases:

one showing a blank page, causing by the bug.

and the other with a modification so it displays correctly.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2005-10-18 19:43:54 PDT
This is a regression from bug 312097.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2005-10-18 19:44:23 PDT
Created attachment 200034 [details] [diff] [review]
Patch

Only notify up front on the _first_ frameset we see.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2005-10-18 19:45:08 PDT
We should get this in on branch.  Stupid logic snafu.  :(
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2005-10-18 21:59:55 PDT
Created attachment 200045 [details] [diff] [review]
Better patch

Found some other cases of bug 312097 that weren't really fixed by the patch in
that bug.  I've gone through and made the first (and only the first) frameset
act exactly like body in all the cases that matter now...
Comment 11 Asa Dotzler [:asa] 2005-10-19 11:47:42 PDT
please request approval for this patch when it's landed and verified on the
trunk. Thanks.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-19 15:42:52 PDT
Comment on attachment 200045 [details] [diff] [review]
Better patch

r+sr=jst
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2005-10-19 20:11:14 PDT
Fixed on trunk.
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2005-10-19 20:12:41 PDT
Comment on attachment 200045 [details] [diff] [review]
Better patch

Requesting 1.8 branch approval.  This patch basically has two parts:

1)  fix a logic bug in the fix for bug 312097, since the parser allows multiple
framesets in a page, unlike bodies
2)  Make more of the frameset code like the body code from the point of view of
notifications.

I've checked that this doesn't regress any of the other frameset bugs involved,
or anything else I can find.
Comment 15 Asa Dotzler [:asa] 2005-10-20 11:12:29 PDT
Let's get a trunk verification here. 
Comment 16 Marcia Knous [:marcia - use ni] 2005-10-20 17:08:20 PDT
Verified using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1)
Gecko/20051020 Firefox/1.6a1.  The URL cited displays fine as does the test case.
Comment 17 Frank Yan (:fryn) 2005-10-20 17:59:09 PDT
i received this notification:


asa@mozilla.org changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #200045 [details] [diff]|approval1.8rc1?             |approval1.8rc1+
              Flag|                            |


but this page itself hasnt updated?
Comment 18 Frank Yan (:fryn) 2005-10-20 18:16:17 PDT
never mind, that took a while though.
Comment 19 Frank Yan (:fryn) 2005-10-20 18:19:33 PDT
so it's been checked into the trunk but not the branch?
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 20:01:44 PDT
Fixed on branch.

studio17, the branch checkin had to wait on branch approval.

Note You need to log in before you can comment on or make changes to this bug.