Closed Bug 246012 Opened 17 years ago Closed 17 years ago

[FIXr]M17 Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ]

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P1)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta1

People

(Reporter: joost, Assigned: bzbarsky)

References

Details

(5 keywords)

Crash Data

Attachments

(5 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608 Firefox/0.8.0+

When using *[src*="?ad"] { display: none !important }  to hide ads in
userContent.css firefox and mozilla suite ( FF 0.9rc and Mozilla Suite 1.7rc3)
crash when an iframe is present containing :
<iframe src="?ad=1"></iframe> 

Reproducible: Always
Steps to Reproduce:
1. Add stylesheet rule to userContent.css
2. load page with iframe
3. press f6

Actual Results:  
The browser crashed.


I've entered a report in the talkback agent (id TB81055W) and made a testcase.
Both mozilla suite en firefox were tested with a clean profile.
Attached file Testcase
Added testcase with userContent style rule, and html containing the iFrame
Attached file TBTB81055W data
Keywords: crash
Summary: Crash when pressing f6 on page with iframe and applied custom CSS style → Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ]
Attached file online testcase 2
In the firefox bug forum mw reduced the testcase even further. Only press F6 to
crash the browser. Seems like the problem is display: none; in combination with
a src="?a" 
it won't crash with only src="?" or only src="a", however it will crash when
replacing the a with some more random characters.
also crashing on Linux using FF trunk 20040604: TB82028.
OS: Windows XP → All
(In reply to comment #4)
> Created an attachment (id=150348)
> online testcase 2

Hmm..this is weird. The local html file i used for this testcase crashes when
pressing F6, but the version here on bugzilla won't crash. The code is exactly
the same.
OS: All → Windows XP
OS: Windows XP → All
Sounds like a dupe of bug 146255 (same callstack) but that one is supposed to be
fixed.
The online testcase 2 does not crash on me, using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/2004-06-01
Firefox/0.8.0+
But it does crash, using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/2004-06-02
Firefox/0.8.0+
online testcase 2 crashes here when i save it to a local file with firefox 0.9rc
and mozilla suite 1.7rc3 (Without  the css rule in userContent.css) 
It doesn't crash in both browser when i use the online testcase online here. 

However, when i add the css rule in userContent.css it crashes both browsers
(online and local)
This is probably the same issue as bug 146255, just with a different docshell...
Assignee: general → events
Status: UNCONFIRMED → NEW
Component: Browser-General → Event Handling
Ever confirmed: true
QA Contact: general → ian
Attached patch This should fix it (obsolete) — Splinter Review
I can't actually reproduce this crash in my devel build; can someone seeing the
crash test the patch?
Attachment #150365 - Flags: superreview?(jst)
Attachment #150365 - Flags: review?(jst)
Comment on attachment 150365 [details] [diff] [review]
This should fix it

 PRBool
 nsEventStateManager::IsIFrameDoc(nsIDocShell* aDocShell)
 {
+  // XXXbz wouldn't it be better to use our window's owning content
+  // and not deal with all this mess?

Um, yes, it would...

+  // The parent docshell may not have a presshell, so just get the
+  // document directly.
+  nsCOMPtr<nsIDocument> parentDoc = do_GetInterface(parentItem);
+  if (!parentDoc) {
+    NS_ERROR("We're a child of a docshell without a document?");
+    return PR_FALSE;
+  }

Unless I missed something, do_GetInterface() to an nsIDocument on a docshell
will always return null. nsIDOMDocument would work, but not nsIDocument.

r+sr=jst with the above fixed.
Attachment #150365 - Flags: superreview?(jst)
Attachment #150365 - Flags: superreview+
Attachment #150365 - Flags: review?(jst)
Attachment #150365 - Flags: review+
I'm not going to get a chance to really build-and-test this until July 11 at
this point... if someone can start with the attached patch and adjust it per
jst's review, that would be great.
(In reply to comment #11)
> I can't actually reproduce this crash in my devel build; can someone seeing the
> crash test the patch?
I've applied the patch and rebuild my firefox build.
It doesn't crash anymore with the patch applied.
Instead, I get an ASSERTION: We're a child of a docshell without a document?:
'Error', file C:/mozilla/mozilla/content/events/src/nsEventStateManager.cpp,
line 5204

F6 doesn't seem to select the url bar anymore with the patched build in this
testcase.

By the way, the testcase does seem to crash also the 1.7 branch. At least it
crashes with me, using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/2004-06-08
Firefox/0.8.0+
Adding topcrash keyword since I see a few of these crashes in early M17 Talkback
data and it's easily reproducible.
Keywords: topcrash
Summary: Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ] → M17 Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ]
Keywords: testcase
Flags: blocking1.8a2?
Attachment #150365 - Flags: approval1.7.1?
Flags: blocking1.8a2?
Attachment #150365 - Attachment is obsolete: true
Comment on attachment 153244 [details] [diff] [review]
Patch updated to comments

jst, how's this?
Attachment #153244 - Flags: superreview?(jst)
Attachment #153244 - Flags: review?(jst)
Comment on attachment 153244 [details] [diff] [review]
Patch updated to comments

r+sr=jst
Attachment #153244 - Flags: superreview?(jst)
Attachment #153244 - Flags: superreview+
Attachment #153244 - Flags: review?(jst)
Attachment #153244 - Flags: review+
Attachment #150365 - Flags: approval1.7.2?
Taking.
Assignee: events → bzbarsky
Priority: -- → P1
Summary: M17 Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ] → [FIXr]M17 Crash when pressing f6 on page with iframe and applied custom CSS style [@ nsEventStateManager::IsIFrameDoc ]
Target Milestone: --- → mozilla1.8beta
Checked in for 1.8a3 or 1.8b or whatever this next milestone is.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment on attachment 153244 [details] [diff] [review]
Patch updated to comments

I think this is worth taking on the 1.7 branch.
Attachment #153244 - Flags: approval1.7.2?
Comment on attachment 153244 [details] [diff] [review]
Patch updated to comments

a=mkaply - what about aviary?
Attachment #153244 - Flags: approval1.7.2? → approval1.7.2+
Fixed on 1.7 branch.
Keywords: fixed1.7.2
Anything with 1.7 branch approval has automatic approval for aviary, doesn't 
it?  I guess someone will sync up aviary with the 1.7.2 changes at some point 
anyway...
Whiteboard: needed-aviary1.0
Given bug 254539 affecting Firefox and maybe dupe of this one, nominating for
Aviary 1.0PR (in addition to the whiteboard, sorry if it's redundant) to make
sure it's in the scope when quering all blocking-aviary bugs.
Flags: blocking-aviary1.0PR?
*** Bug 254539 has been marked as a duplicate of this bug. ***
Keywords: fixed-aviary1.0
Whiteboard: needed-aviary1.0
Flags: blocking-aviary1.0PR?
*** Bug 255254 has been marked as a duplicate of this bug. ***
Crash Signature: [@ nsEventStateManager::IsIFrameDoc ]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.