Closed Bug 312704 Opened 19 years ago Closed 18 years ago

crash [@ nsNodeInfoManager::GetTextNodeInfo] on ebay page with JavaScript off

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: ch.ey, Assigned: mrbkap)

Details

(Keywords: crash, fixed1.8.0.7, fixed1.8.1)

Crash Data

Attachments

(3 files)

Crash after clicking on "Continue>" button on ebay's "Review Your Purchase"
after selecting a payment method. It crashes when status bar displays "read
secureinclude.ebaystatic.com".

But it only crashes if JavaScript is switched *off* (javascript.enabled = false)
or is prohibited for the page via policies.

It crashes with Firefox Trunk Win 20051016, Seamonkey Trunk Win 20051013 and
Seamonkey Trunk Linux 20051016.

Talkback ID's for this are:
Seamonkey Win:
TB10733074Y, TB10733077G, TB10733304Z, TB10733396Q, TB10734827Y, TB10735665G
Firefox Win:
TB10762793X, TB10763001M
From talkback ID TB10733074Y:

nsNodeInfoManager::GetTextNodeInfo 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsNodeInfoManager.cpp,
line 272]
SinkContext::FlushText 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 1816]
SinkContext::FlushText 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 1802]
SinkContext::FlushTags 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 1720]
HTMLContentSink::BeginUpdate 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 3996]
nsXULElement::SetAttr 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 1292]
nsGfxScrollFrameInner::SetAttribute 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 2504]
nsGfxScrollFrameInner::LayoutScrollbars 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 2386]
nsHTMLScrollFrame::Reflow 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsGfxScrollFrame.cpp,
line 833]
nsContainerFrame::ReflowChild 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsContainerFrame.cpp,
line 891]
ViewportFrame::Reflow 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsViewportFrame.cpp,
line 239]
PresShell::InitialReflow 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 2762]
nsContentSink::StartLayout 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsContentSink.cpp,
line 898]
HTMLContentSink::StartLayout 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 3525]
CNavDTD::DidBuildModel 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 598]
Severity: normal → critical
Summary: crash on ebay page with JavaScript off → crash [@ nsNodeInfoManager::GetTextNodeInfo] on ebay page with JavaScript off
Assignee: nobody → general
Component: General → DOM
Keywords: crash
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
Is this still reproducible with current trunk?
(In reply to comment #2)
> Is this still reproducible with current trunk?

Yes, just crashed again with 2006060110, see TB19557959M.
Hmm, ok.  What are the steps to reproduce?  Assume that you're telling someone who has never used eBay (good assumption in this case) and who doesn't want to spend time hunting all over the eBay UI for the right things to click (also a good assumption).

Is an eBay account or something like that needed to reproduce?
(In reply to comment #4)
> Hmm, ok.  What are the steps to reproduce?  Assume that you're telling someone
> who has never used eBay (good assumption in this case) and who doesn't want to
> spend time hunting all over the eBay UI for the right things to click (also a
> good assumption).
> 
> Is an eBay account or something like that needed to reproduce?

Unfortunatelly yes, since it happens in the confirm shipping and payment steps after buying something.
But I'll nevertheless describe the steps:
1. After buying something, visit the items page.
2. Click on the "Pay now" button on the top of the page
3. On the next "Review Your Purchase" page, click on "Continue" button on the bottom of the page
4. Boom

Again, only if JS is off.
I suspect that fixing these asserts will fix the crash.
Attached patch Proposed fixSplinter Review
This should fix the crashes by not allowing <noscript> to be a child of the <html>. Instead, we'll open the <head> for it, and the <link> will be processed in the same context as the noscript.
Attachment #224781 - Flags: review?(bzbarsky)
Attachment #224776 - Flags: superreview?(bugmail)
Attachment #224776 - Flags: review?(mrbkap)
Assignee: general → mrbkap
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Comment on attachment 224781 [details] [diff] [review]
Proposed fix

Looks good, and fixes the crash.

I think we want to take this on branches...
Attachment #224781 - Flags: review?(bzbarsky) → review+
Attachment #224776 - Flags: review?(mrbkap) → review+
Status: NEW → ASSIGNED
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
Attachment #224781 - Flags: superreview?(bugmail)
Attachment #224776 - Flags: superreview?(bugmail) → superreview+
Comment on attachment 224781 [details] [diff] [review]
Proposed fix

I am very worried though that we'll get a repeat of the <object> crashes. But if you say that things should be more stable now...

Actually, maybe it'd be a good idea to reenable <object> in head since that'd give better testing to the same codepaths.
Attachment #224781 - Flags: superreview?(bugmail) → superreview+
Fix checked into the trunk. Boris will have to remember to check the assertion fix in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Checked in the assertion.
What do we want to do about the branch here? The patch is fairly risky given that similar changes for <object> caused crashes. Are the fixes for the <object> related problems checked in to all braches?

Could we maybe disallow noscript inside <head> in addition to disallowing the 'anywhere' flag? At least for branch?

In any case, anything we want for 1.8.0.x would have to go on 1.8.1 first to allow baking.
Depends on: 341320
No longer depends on: 341320
No answer to comment 13, no 1.8 branch landing, looks like a minus for this release.
Flags: blocking1.8.0.5? → blocking1.8.0.5-
Flags: blocking1.8.0.6?
Flags: blocking1.8.1? → blocking1.8.1+
Did you want to request 1.8 branch approval?
I checked this patch + the patch for bug 341359 into the 1.8 branch.
Keywords: fixed1.8.1
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Comment on attachment 224781 [details] [diff] [review]
Proposed fix

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #224781 - Flags: approval1.8.0.7+
Fixed on the 1.8.0 branch.
Keywords: fixed1.8.0.7
I was unable to generate the original crash.  Could the creator of this bug please attempt to verify the fix with a nightly build?
Crash Signature: [@ nsNodeInfoManager::GetTextNodeInfo]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: