Make quirks mode not validate createElement's tagName argument to preserve 1.4 compat

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
15 years ago
3 months ago

People

(Reporter: doronr, Assigned: sicking)

Tracking

({fixed1.7})

Trunk
x86
All
Points:
---
Bug Flags:
blocking1.7 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-aviary1.0)

Attachments

(2 attachments)

Reporter

Description

15 years ago
I've seen several cases where web applications have broken because 1.4 was
linient and 1.7 isn't any more.

So we probably want to allow the old behavior in quirks mode in createElement at
least, and probably others as well.
Reporter

Comment 1

15 years ago
Assignee: general → doronr
Status: NEW → ASSIGNED
Comment on attachment 149775 [details] [diff] [review]
Adds quirks check

r+sr=jst for this change to get back backwards compatibility.
Attachment #149775 - Flags: superreview+
Attachment #149775 - Flags: review+
Reporter

Comment 3

15 years ago
cc: peterv
Sigh. So what invalid names were they using then?
Reporter

Comment 5

15 years ago
things like "<div>".

Reporter

Comment 6

15 years ago
I hate doing this too, any reasons to not do this? (other than standards :)
Reporter

Updated

15 years ago
Flags: blocking1.7?
Comment on attachment 149775 [details] [diff] [review]
Adds quirks check

a=mkaply for 1.7.

This is a big IBM deal
Attachment #149775 - Flags: approval1.7+
Fix in trunk and 1.7. Put it on aviary too.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: blocking1.7? → blocking1.7+
Keywords: fixed1.7
Resolution: --- → FIXED

Updated

15 years ago
Whiteboard: fixed-aviary1.0
Doron: does these applications only use things like '<div>'? In IE that will
actually create an element named 'div' so maybe we could emulate that behaviour
rather then just allowing any tagname?

IE will actually let you do stuff like |document.createElement('<div
foo=bar>')|, but I don't think we need to emulate that. But maybe we could
restrict to allowing  enclosing '<' '>'? Would that work for your applications?

The reason for this is that i'd like to avoid stuff like spaces and other evil
characters in tagnames.
Reporter

Comment 10

14 years ago
(In reply to comment #9)
> Doron: does these applications only use things like '<div>'? In IE that will
> actually create an element named 'div' so maybe we could emulate that behaviour
> rather then just allowing any tagname?
> 
> IE will actually let you do stuff like |document.createElement('<div
> foo=bar>')|, but I don't think we need to emulate that. But maybe we could
> restrict to allowing  enclosing '<' '>'? Would that work for your applications?
> 
> The reason for this is that i'd like to avoid stuff like spaces and other evil
> characters in tagnames.

For the "<div>" case, creating an HTMLUnknownElement is fine usually.  I don't
think adding specific support is worth it - the issue of this bug is that we
changed behavior, not to support this evil thing.
The change was a good one though, and one that i'd like to make. So my question
is, could we use an algorithm like this:

1. if name starts with '<' and ends with '>' strip those characters
2. validate name according to spec

This would allow '<div>' but not '#foo', '<foo', 'foo bar' or '<foo bar>'
Please make this quirks-only, like the current patch.
Tank ewe!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Doron: will this keep your precious apps working? Since working apps is that
important to you. I mean, comon, there's a whole world out there that doesn't
depend on it. Spend time with an old friend, visit a distant relative, take up
monoskiing.

This will make createElement('<div>') keep working, while making sure we never
have any unexpected characters in tagnames.
Assignee: doronr → bugmail
Status: REOPENED → ASSIGNED
Attachment #185261 - Flags: superreview?(bzbarsky)
Attachment #185261 - Flags: review?(doronr)
Comment on attachment 185261 [details] [diff] [review]
Alternative fix

sr if things don't break.  ;)
Attachment #185261 - Flags: superreview?(bzbarsky) → superreview+
Reporter

Comment 17

14 years ago
Comment on attachment 185261 [details] [diff] [review]
Alternative fix

I never cared for supporting the apps, I just cared that we try to preserve
backward compat (because, you know, it matters or something).

Change looks alright, if anyone does createElement("<div id='foo'>"), the app
won't work anyways as intended.
Attachment #185261 - Attachment is obsolete: true
Attachment #185261 - Flags: review?(doronr) → review+
Comment on attachment 185261 [details] [diff] [review]
Alternative fix

Doron: I was, of course, ironic. We defenetly want to support important apps,
that's why we have quirks mode after all.

You didn't intend to mark this obsolete did you?

I'll hold this until 1.9 opens since the patches I have that depend on it won't
land until then anyway.

It'd be good to get this in beta to ensure that
Attachment #185261 - Attachment is obsolete: false
erm.. so just ignore that last half-sentence :)
Comment on attachment 185261 [details] [diff] [review]
Alternative fix

Eh, wtf, we might as well land this for 1.1. Ver few people should be using the
attribute syntax since that's never worked anyway, and for the rest this patch
should actually be an improvement.
Attachment #185261 - Flags: approval1.8b3?
Reporter

Comment 21

14 years ago
My comment got cut off, sorry for obsoleting, must have been vodoo.  And I left
my irony detector off.

And yes, this should make 1.8, we really want to stabalize our behavior rather
tha n change it for each release :)

Updated

14 years ago
Attachment #185261 - Flags: approval1.8b3? → approval1.8b3+
Checked in
Status: ASSIGNED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.