Closed Bug 352723 Opened 18 years ago Closed 18 years ago

e4x parameter is inverted in <xul:script>

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: nanto, Assigned: asqueella)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files, 3 obsolete files)

In XUL script element, parameter "e4x=1" disables XML option and "e4x=0" enables. Maybe regression of bug 255942.

See the testcase.
Attached file testcase (obsolete) —
Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
application/javascript ... E4X comment literal is recognized
application/javascript; e4x=1 ... E4X comment literal is recognized
application/javascript; e4x=0 ... E4X comment literal is not recognized
application/javascript; version=1.6 ... E4X comment literal is recognized

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060914 Minefield/3.0a1
application/javascript ... E4X comment literal is not recognized
application/javascript; e4x=1 ... E4X comment literal is not recognized
application/javascript; e4x=0 ... E4X comment literal is recognized
application/javascript; version=1.6 ... E4X comment literal is recognized
What about in BonEcho (Firefox 2) beta 2 or nightly?

/be
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20060914 BonEcho/2.0
application/javascript ... E4X comment literal is recognized
application/javascript; e4x=1 ... E4X comment literal is recognized
application/javascript; e4x=0 ... E4X comment literal is not recognized
application/javascript; version=1.6 ... E4X comment literal is recognized
Yep, the patch in bug 255942 got the logic backwards. Thanks for the nice testcase. Patch coming up.
Assignee: nobody → asqueella
Blocks: dom-agnostic
Keywords: regression, testcase
OS: Windows XP → All
Hardware: PC → All
Attached file expanded testcase
Attachment #238494 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
This makes us behave exactly like Firefox 2. Is this desired?
Attachment #239852 - Flags: superreview?(brendan)
Attachment #239852 - Flags: review?(brendan)
Attached patch patch (obsolete) — Splinter Review
Meh, wrong file. I need to sleep.
Attachment #239852 - Attachment is obsolete: true
Attachment #239853 - Flags: superreview?(brendan)
Attachment #239853 - Flags: review?(brendan)
Attachment #239852 - Flags: superreview?(brendan)
Attachment #239852 - Flags: review?(brendan)
Comment on attachment 239853 [details] [diff] [review]
patch

>+          if (nsParserUtils::IsJavaScriptLanguage(lang, &version)) {
>+              langID = nsIProgrammingLanguage::JAVASCRIPT;

A blank line here would yield more readable code IMHO.

>+              // Even when JS version < 1.6 is specified, E4X is
>+              // turned on in XUL.
>+              version |= JSVERSION_HAS_XML;
>+          }

r+sr=me, thanks for fixing this.

/be
Attachment #239853 - Flags: superreview?(brendan)
Attachment #239853 - Flags: superreview+
Attachment #239853 - Flags: review?(brendan)
Attachment #239853 - Flags: review+
Attached patch for checkinSplinter Review
You're right.
Attachment #239853 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Checked in by smaug.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Flags: in-testsuite?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: