Closed Bug 619644 Opened 13 years ago Closed 13 years ago

Javascript confirm prompt breaks document.write iff closed with ESC (fine if cancel clicked)

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b9
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: kdevel, Assigned: khuey)

References

Details

(Keywords: regression)

Attachments

(3 files)

User-Agent:       
Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b9pre) Gecko/20100101 Firefox/4.0b9pre

document.write after Pressing the ESC key in confirm dialog overwrites document.

Reproducible: Always

Steps to Reproduce:
1. open testcase https://bugzilla.mozilla.org/attachment.cgi?id=494685
2. press ESC
3.
Actual Results:  
"v = false"

Expected Results:  
"Lorem ipsum ... 
Do you agree?
v = false"

Built from http://hg.mozilla.org/mozilla-central/rev/8755d308796d
Attached file Reduced testcase
Confirmed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101216 Firefox/4.0b9pre ID:20101216030331

Have reduced the testcase and so updated the STR to match.

New STR:
1) Open reduced testcase (attachment 498236 [details]) in new tab.
2) Press escape.
3) Observe content shown.
4) Repeat steps 1-3, except this time click cancel rather than pressing escape.

Expected:
- Same result regardless of escape or cancel - ie: Both "Original Content" and "Extra content" messages shown on the page.

Actual:
- If cancel clicked: see expected.
- If escape used, only the "Extra Content" message is shown, the "Original Content" message is overwritten.

Does not occur using 3.6.13, so is a regression in Fx4.

Marking as such and finding the regression window now.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Linux → All
Version: unspecified → Trunk
Last good nightly: 2010-11-19 
First bad nightly: 2010-11-20

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6478c1b83d60&tochange=baa6cc2f72e4

Suspect: Bug 59314 - Alerts should be content-modal, not window-modal

Provisionally marking as blocking that bug for visibility over there.

Adjusting title to clarify issue.
Blocks: 59314
status2.0: --- → ?
Product: Firefox → Toolkit
QA Contact: general → general
Hardware: x86_64 → All
Summary: confirm dialog: strange ESC-behavior (1) → Javascript confirm prompt breaks document.write iff closed with ESC (fine if cancel clicked)
blocking2.0: --- → ?
status2.0: ? → ---
Blocks: 619645
Blocks: 619646
Very odd, not sure what's going on here. Think we need to block on figuring it out.
blocking2.0: ? → betaN+
Attached patch PatchSplinter Review
We need to stop the ESC from propagating up to the handler in browser.js which aborts the document load.
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #499650 - Flags: review?(dolske)
While fixing this I also discovered that going backwards when a tab-modal prompt is visible causes a compartment mismatch assertion.  I filed Bug 621297 for that.
Your patch does not fix the "underlaying" problem in its totality:
STR

1. open reduced testcase -> spinner spins
2. press stop-button with mouse (stop loading page) -> spinner stops
3. press ESC key -> 
   a) original content vanishes
   b) spinner spins
Ooops. Someone asked me to create a separate bug out of every symptom. So my previous Comment #9 hat to go to 619645 or do I have to file a new bug because the STR has changed??
(In reply to comment #10)
> Ooops. Someone asked me to create a separate bug out of every symptom. So my
> previous Comment #9 hat to go to 619645 or do I have to file a new bug because
> the STR has changed??

That's generally the right thing to do.  We can then dupe them once we're sure that they're the same underlying cause.

(In reply to comment #9)
> Your patch does not fix the "underlaying" problem in its totality:
> STR
> 
> 1. open reduced testcase -> spinner spins
> 2. press stop-button with mouse (stop loading page) -> spinner stops
> 3. press ESC key -> 
>    a) original content vanishes
>    b) spinner spins

So this is a slightly different issue.  Here we need to kill the prompt and all running scripts when stop is clicked.  Lets take this to a new bug (please post the number here though).

Thanks for your help with this!
The new issue is in Bug 621308.
Comment on attachment 499650 [details] [diff] [review]
Patch


>+// This is a little yucky, but it works
>+const expectedFinalDoc = 
>+"<head><\/head><body><p>Original content<\/p> ....

Add a comment here noting that this is the content of bug619644_inner.html.

Also, if you add a <head> and <body> to that file, does it then exactly match the string you're getting here? That would be nice just for clarity.
Attachment #499650 - Flags: review?(dolske) → review+
(And thanks for the patch and omgtest!)
http://hg.mozilla.org/mozilla-central/rev/b2275b45a33d

(In reply to comment #13)
> Comment on attachment 499650 [details] [diff] [review]
> Patch
> 
> 
> >+// This is a little yucky, but it works
> >+const expectedFinalDoc = 
> >+"<head><\/head><body><p>Original content<\/p> ....
> 
> Add a comment here noting that this is the content of bug619644_inner.html.

Done

> Also, if you add a <head> and <body> to that file, does it then exactly match
> the string you're getting here? That would be nice just for clarity.

I believe so, but I pushed without changing that since I don't have time to compile a tree with this patch on this machine at the moment.  I'll investigate/fix soon.

(In reply to comment #14)
> (And thanks for the patch and omgtest!)

:-)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [khuey see comment 15]
Target Milestone: --- → mozilla2.0b9
New testcase: Kind of regression?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
STR:

1. open testcase
2. press button
3. press esc or press cancel or press OK

actual result:
keeps spinning.

expected result:
no spinning
That's by design.  document.write after the load is complete replaces the document.  It doesn't make much sense, but that's how it worked in the past so that's how it works now.

http://www.whatwg.org/specs/web-apps/current-work/multipage/apis-in-html-documents.html#document.write%28%29
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
And the still spinning part is a dupe of one of the other bugs (the "aborting the load has issues" one)
@Kyle: Can't see any open bugs which cover this issue. Do you have a bug id?
(In reply to comment #20)
> And the still spinning part is a dupe of one of the other bugs (the "aborting
> the load has issues" one)
Bug 621308 presumably?
Oh, that happens with cancel and OK?  That's a diffferent issue, please file separately and CC me.
Bug 623880 filed
(In reply to comment #13)
> Also, if you add a <head> and <body> to that file, does it then exactly match
> the string you're getting here? That would be nice just for clarity.

Done in http://hg.mozilla.org/mozilla-central/rev/8ba73a8038a9
Whiteboard: [khuey see comment 15]
Depends on: 625038
Blocks: 625038
No longer depends on: 625038
You need to log in before you can comment on or make changes to this bug.