Closed Bug 201108 Opened 22 years ago Closed 22 years ago

<BODY onLoad>, |location.href| for other frame(s) appears ignored at first

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.4final

People

(Reporter: sgautherie, Assigned: radha)

References

Details

(Keywords: regression, topembed+, Whiteboard: [adt2])

Attachments

(2 files)

Reduced Testcase (RTC_SG_1)
1.02 KB, application/zip
Details
quick patch to docshell
1.78 KB, patch
danm.moz
: review+
darin.moz
: superreview+
asa
: approval1.4+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4a) Gecko/20030401 Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4a) Gecko/20030401 First of all, let's say that this is a regression which happened between "Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.3) Gecko/20030312" and "Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4a) Gecko/20030401" ! {It's the first time that I need to install back a previous release to check for sure :-(} This had allways (= for *many* releases at least) been working before v1.4a. (Netscape v4.8 and previous has allways worked too, but that may be another story.) The relevent source code extract (from a frame) is [ <body onload="if (top.location == self.location) {self.location = 'https://www.net751.caisse-epargne.fr/private/ident.asp';}else load3(); defaultStatus=''; return true"> <script LANGUAGE="JavaScript"> <!-- function load1 () { parent.gauche.location.href='/private/vide_g.asp'; parent.haut.location.href='/private/vide_h.asp'; } function load2H () { parent.gauche.location.href='/private/frame2_g.asp'; parent.haut.location.href='/private/vide_h.asp'; } function load2V () { parent.gauche.location.href='/private/vide_g.asp'; parent.haut.location.href='/private/frame2_h.asp'; } function load3 () { parent.gauche.location.href='/private/frame2_g.asp'; parent.haut.location.href='/private/frame2_h.asp'; } //--> </SCRIPT> ] There is no error in the JavaScript Console; and I can't tell if it is related to one of the many bug filed about the "onLoad()" event, or if it is something (JavaScript !?) else which has gone wrong. Bottom line is that it prevents me from loading the menus, which I miss, at least because they contain the button to logout of the "site session". Reproducible: Always Steps to Reproduce: (I see this on an banking site, and I can't provide my private access...) Nevertheless, see the results below, as observed on screen and confirmed by a network (proxy) activity capture. Actual Results: In v1.4a, it's as if 'load1()' is executed, which should not. Expected Results: In v1.3, it's as if 'load3()' is executed, which is expected. Feel free to change the component and the summary as needed. I could generate "nsHttp:5" logs if you want them. If you have a guess on the date of regression, I'd be willing to try {for the first time} 1+ nightly (v1.4a) builds as needed.
Keywords: regression
Summary: BODY OnLoad, in a frame, does the wrong action... (that the initial case) → BODY OnLoad, in a frame, does the wrong action... (that's the initial case)
Flags: blocking1.4b?
Is there a page this could be tested on?
Not on the actual site (private access) :-(
In that case, we're going need a lot more of the source code to see what's going on -- use http://bugzilla.mozilla.org/attachment.cgi?bugid=201108&action=enter please...
1. Unzip the 4 files in the same directory; 2. Load the 'index.asp.html' file; (I added 2 alert()) 3. On the right frame, use 'This Frame > Reload Frame'; 4. Check that the left frame has changed. (Use 'Back/Forward' to best see it)
OK, that's calling the right function but the location change is not happening... not sure why. jst?
Assignee: asa → dom_bugs
Component: Browser-General → DOM Level 0
OS: Windows 95 → All
QA Contact: asa → ashishbhatt
Hardware: PC → All
Re comment 3: (and addition to attachment in comment 4) I feared you would say that; But lucky you: I spent some time retrieving the various files {frameset in frameset, post, redirect: who can design such a site :-<}; Only to spend some more time narrowing down the relevent code. You won't be disappointed: I left a few lines only :-> First conclusion is that 'load3()' is allways executed, only it fully work on Frame Reload only. NB: On the actual site, even the Frame Reload workaround won't fix anything ... I just hope that fixing the local testcase behaviour will fix the actual site behaviour too.!. (== this will have to be checked with a HTTP server too, in a second time.) Netscape v4.8: allways updates (between the 2 alert()) Mozilla v1.3: allways updates (after the 2nd alert()) Now, I can let you try and find at which build, and why, this bug appeared :->
Keywords: 4xp
Summary: BODY OnLoad, in a frame, does the wrong action... (that's the initial case) → BODY OnLoad, in a frame ... location.href does nothing (in the reported case)
Blocks: 201159
Blocks: 201336
Blocks: 201337
Upping severity. This is getting reported a _lot_ with 1.4a. When I get home tonight I'll look for when the regression happened.
Severity: major → critical
OK, this broke between build 2003-03-13-08 (which works fine) and build 2003-03-14-05 (which is broken). Looking at the checkins in that window...
No longer blocks: 201336
No longer blocks: 201337
Blocks: 200676
No longer blocks: 200676
OK, locally backing out bug 166736 fixes this bug on the bug this blocks. Over to session history, ccing the various reviewers and such. I bet we drop the location change on the floor because we're in the middle of a load or some such...
Assignee: dom_bugs → radha
Component: DOM Level 0 → History: Session
QA Contact: ashishbhatt → petersen
I wouldn't want the other patch to bug 166736 to be backed out, since that takes care of a more common basic problem. I will look into this when I get a chance.
Right, I was not saying we should back it out; just stating what led me to think it was definitely the culprit. ;) Thanks for looking into this, Radha.
Addition to comment 6 (and confirmation of comment 11): On the actual 'net751' site, The Reload trick is helpless; But I discovered that using Back, does its job AND the frames then display there previously intended content :-) This seems definately related to bug 166736 about "History: Session".
Summary: BODY OnLoad, in a frame ... location.href does nothing (in the reported case) → BODY onLoad, location.href for other frame(s) appears ignored at first
Precision on comment 12: The steps are: (Login) -> 1st frameset display with some empty frames -> click on a link to change the frame which loaded fine -> new frame, still empty others -> Back -> get all initials frames with there intended content :-)
Re comment 9: Which bug report is "fixed" by the back out ? The current one (201108) and/or bug 201159 ? If both, bug 201159 could be marked as duplicate; If one only, then we would know that there are at least two issues to fix.
> Which bug report is "fixed" by the back out ? Both. But since the fix will _not_ be to back it out, I'd like to keep the bugs separate in case we end up fixing one but not the other (hopefully, that will not happen).
Blocks: 202258
Flags: blocking1.4b? → blocking1.4b+
If you want another site exhibiting this problem: http://xa2.loran.com/nm/find/ login=demo password=guide I have reverted back to moz 1.3 for now.
adt: nsbeta1+/adt2
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt2]
Flags: blocking1.4b-
Flags: blocking1.4b+
Flags: blocking1.4+
ADT: Nominating topembed
Keywords: topembed
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4b) Gecko/20030507] Bug still there. (with v1.3 profile, at least)
Keywords: topembed → topembed+
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.3.1) Gecko/20030425] v1.3.1 works fine, as v1.3 does. (with v1.3 profile, at least)
so this might be one of the few remaining higher visability regressions left open from the 1.0 branch that need to be cleaned up for 1.4 final. what do we need to do here? who can help out?
I think rahda is out until the end of the month.
bzbarsky (or anyone who has a latest tree and interested in this bug) , I need your help here. Looking thro' the changes to the other bug, it looks like it is this following segment from the patch to bug 166736, that is causing this bug: @@ -648,31 +646,28 @@ // This is a pre-existing subframe. If the load was not originally initiated // by session history, (if (!shEntry) condition succeeded) and mCurrentURI is not null, // it is possible that a parent's onLoadHandler or even self's onLoadHandler is loading - // a new page in this child. Check parent's and self's busy status and if it is, + // a new page in this child. Check parent's and self's onLoadHandler flag and if it is set, // we don't want this onLoadHandler load to get in to session history. - PRUint32 parentBusy=BUSY_FLAGS_NONE, selfBusy = BUSY_FLAGS_NONE; - parentDS->GetBusyFlags(&parentBusy); - GetBusyFlags(&selfBusy); - if (((parentBusy & BUSY_FLAGS_BUSY) || (selfBusy & BUSY_FLAGS_BUSY)) && shEntry) { - // we don't want this additional load to get into history, since this - // load will automatially happen everytime, no matter how the page is loaded. + PRBool parentInOnLoadHandler=PR_FALSE, selfInOnLoadHandler = PR_FALSE; + parentDS->IsExecutingOnLoadHandler(&parentInOnLoadHandler); + IsExecutingOnLoadHandler(&selfInOnLoadHandler); + if ((parentInOnLoadHandler || selfInOnLoadHandler) && shEntry) { loadType = LOAD_NORMAL_REPLACE; shEntry = nsnull; . Can you just undo this part of my checkin and see if it fixes the problem. It is 99% the above segment that is causing this bug OR the following one: @@ -615,14 +615,12 @@ if (shEntry && (parentLoadType == LOAD_NORMAL || parentLoadType == LOAD_LINK)) { // The parent was loaded normally. In this case, this *brand new* child really shouldn't // have a SHEntry. If it does, it could be because the parent is replacing an - // existing frame with a new frame, probably in the onLoadHandler. We don't want this + // existing frame with a new frame, in the onLoadHandler. We don't want this // url to get into session history. Clear off shEntry, and set laod type to // LOAD_BYPASS_HISTORY. - PRUint32 parentBusy=BUSY_FLAGS_NONE; - parentDS->GetBusyFlags(&parentBusy); - if (parentBusy & BUSY_FLAGS_BUSY) { - // The parent is still busy. We most likely got here - // through onLoadHandler. + PRBool inOnLoadHandler=PR_FALSE; + parentDS->IsExecutingOnLoadHandler(&inOnLoadHandler); + if (inOnLoadHandler) { loadType = LOAD_NORMAL_REPLACE; shEntry = nsnull; Can someone try this out and update me. I don't have a latest tree nor consistent cycles to verify the above. Thanks.
yes, backing out the first chunk in comment 23 fixes this bug.
This patch backs off part of the checkin for bug 166736. I have not tested it nor successfully compiled it in my outdated tree. I would like someone to stress test it for me. I think this will take care of both this bug and 166736. Thanks.
Comment on attachment 123995 [details] [diff] [review] quick patch to docshell I don't grok all the subtleties going on here. The offending patch (among other things) set loadType to LOAD_NORMAL_REPLACE if a URI load was fired off in a subframe a little later in the load process (and introduces a whole new accessor which seems to me to be merely another flavour of GetBusyFlags and I find that confusing). This patch reverts that part of the offending patch. I'm guessing this patch partially regresses bug 166736 and the underlying problem is that we're inappropriately multiplexing LOAD_NORMAL_REPLACE. We may need a new flag. Regardless, this patch behaves as expected (in a good way) in every test I tried. In load handlers I've: called focus, called alert, accessed/mutated dom elements and window properties, fired off an image load, opened popup windows, sized the window, set a timer, (from a frame) called a method on the parent to document.write [ into another frame, and into the calling frame HTML containing a load handler of its own ]. All good. I say check it in. (I can check it in for vacationing Radha.)
Attachment #123995 - Flags: review+
...not to mention the patch also fixes the testcase mentioned in this bug. And I just realized I was wasting my time with most of those other tests. The effect of this patch should be very narrow: URI loads fired off in load handlers, which is this bug's complaint. Well that seems to work, too. My review+ remains.
who can super-review this?
Comment on attachment 123995 [details] [diff] [review] quick patch to docshell sr=darin
Attachment #123995 - Flags: superreview+
Comment on attachment 123995 [details] [diff] [review] quick patch to docshell a=asa (on behalf of drivers) for checkin to 1.4
Attachment #123995 - Flags: approval1.4+
Dan, Thanks for all the stress testing. Can you check it in for me?
checked in to trunk (23 May) and 1.4 branch (27 May)
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.4final
Keywords: fixed1.4
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4) Gecko/20030529] (As reporter) I verified that this bug is fixed in v1.4rc1, for both real site (Description) and testcase (comment 6): v1.4rc1 works fine, as v1.3.1 does.
Summary: BODY onLoad, location.href for other frame(s) appears ignored at first → <BODY onLoad>, |location.href| for other frame(s) appears ignored at first
No longer blocks: 201159
*** Bug 201159 has been marked as a duplicate of this bug. ***
No longer blocks: 202258
Verified on Macho /Win32 trunk and branch builds. 2003-06-06-05 - Macho branch,Win32 branch 2003-06-06-08 - Macho trunk, Win32 trunk
Status: RESOLVED → VERIFIED
Keywords: fixed1.4 → verified1.4
Component: History: Session → Document Navigation
QA Contact: chrispetersen → docshell
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: