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

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
DOM
P1
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Christian Eyrich, Assigned: mrbkap)

Tracking

({crash, fixed1.8.0.7, fixed1.8.1})

Trunk
mozilla1.9alpha1
crash, fixed1.8.0.7, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.5 -
blocking1.8.0.7 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
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

Updated

12 years ago
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?
(Reporter)

Comment 3

11 years ago
(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?
(Reporter)

Comment 5

11 years ago
(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.
Created attachment 224771 [details]
Minimal-ish testcase for the asserts I see

I suspect that fixing these asserts will fix the crash.
Created attachment 224776 [details] [diff] [review]
Assertion that catches the first place things go wrong
(Assignee)

Comment 8

11 years ago
Created attachment 224781 [details] [diff] [review]
Proposed fix

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+
(Assignee)

Updated

11 years ago
Attachment #224776 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 11

11 years ago
Fix checked into the trunk. Boris will have to remember to check the assertion fix in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 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.

Updated

11 years ago
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?

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+
Did you want to request 1.8 branch approval?
(Assignee)

Comment 16

11 years ago
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+
(Assignee)

Comment 18

11 years ago
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]
You need to log in before you can comment on or make changes to this bug.