168357 - document.elementID for divs for legacy support

Status: VERIFIED INVALID

(Core :: Layout, defect)

Description

[Doron Rosenberg (IBM)]

We want to kill off 4.x, and the best way is to increase site compatability by
supporting legacy stuff.

I have some xbl that added document.elementID.  This fixes dreameaver 3 created
code.

I also added support for elementID.left/top to fix china floating divs.

Comment 1

[Doron Rosenberg (IBM)]

Attachment: the xbl

quirk.css:

div[id] {
  -moz-binding:
url('chrome://xbl-legacydom/content/xbl-legacydom.xml#legacydom');
}

Comment 2

[Carine Held]

I tested the xbl on 88 sites with floating images:

1) The fix did not introduce additional problems.
2) 23 sites were fixed by the xbl.
3) 19 sites did not have any floating image anymore.
4) 42 sites still had a problematic floating image.

Comment 3

[Brendan Eich [:brendan]]

4xp, right?

/be

Comment 4

[Jason Kersey]

jaimejr was naughty and removed the 4xp.  adding it back.

Comment 5

[Hixie (not reading bugmail)]

Brendan asked me to give my opinion on this, so:

I think it's a waste of time working on extensions when we have plenty of more
important compliance issues, but just like <marquee> and document.all, if people
have written a patch for it and it doesn't affect performance, reduce our
standards compliance, cause regressions, or make the user experience worse, then
sure, whatever.

Comments on the patch coming up.

Comment 6

[David Hyatt]

Random nitpick: The setters should return val to allow for chaining assignment.

Comment 7

[Hixie (not reading bugmail)]

| <bindings id="legacyDOMBindings"
|          xmlns="http://www.mozilla.org/xbl"
|          xmlns:html="http://www.w3.org/1999/xhtml"
|          xmlns:xbl="http://www.mozilla.org/xbl">

Remove the html and xbl namespace prefix declarations, they are not used.


|      <property name="left">
|        <getter>
|          return parseInt(this.style.left,10);
|        </getter>

Is this not broken? style.left should be returning a string with units. You want 
to use the value APIs to get an integer guarenteed to be in pixels.

Ditto with top.


|        <setter>
|          this.style.left = val;
|        </setter>

This is definitely wrong. Either use the value APIs or set the property with
units. (|val + "px"| at a guess)

Ditto with top.

|      <constructor>
|        <![CDATA[
|          // add document.elementID
|		  if (document)
|            document[this.id] = this;
|        ]]>
|      </constructor>

How about if the ID is changed?


|    <content>
|	  <children/>
|    </content>

Remove the <content> block since you are not using it except to insert whitespace.


| quirk.css:
| 
| div[id] {
|   -moz-binding: 
|              url('chrome://xbl-legacydom/content/xbl-legacydom.xml#legacydom');
| }

This must not be in quirks.css, since it does not fulfil the requirements of a
quirk. Just like <marquee>, -moz-binding, and other extensions, it should be
always enabled. Quirks mode is only for things which would otherwise _break_
standards compliant implementations.

Comment 8

[Hixie (not reading bugmail)]

s/integer/number/ (it could be a float)

Comment 9

[Doron Rosenberg (IBM)]

.left/.top are numbers only in 4.x, and various sites  do foo.left + 5 and such
without parseInt

Comment 10

[Carine Held]

Attachment: Old xbl xpi

In case people want to take a look at the old version.

Comment 11

[Bob Clary [:bc] (inactive)]

I have been asked to address the risk of this change. I can only speak to the
effect that it would have on existing sites (not the browser itself).

I do not think this will create new problems for site visitors. 

Sites rarely (never that I can recall) in my experience use document.layerId to
sniff particular browsers. We will only encounter this on sites which already
mistakenly think we are NN4. If the only use of layer related API calls is
document.layerId, then the code will work whereas before it would not -> Net
gain. If they use additional layer related calls then the site will still be
broken however this will not have been the cause -> No change.

Comment 12

[Doron Rosenberg (IBM)]

Attachment: patch, doesn't address nits yet

Comment 13

[Doron Rosenberg (IBM)]

Hixie: so for the getter, does something like this look fine:

      <property name="left">
        <getter>
          var style = window.getComputedStyle(this, null);
          var cssValue = style.getPropertyCSSValue("left");
          var elementLeftValue = cssValue.getFloatValue(CSSPrimitiveValue.CSS_PX);
          return elementLeftValue;
        </getter>
        <setter>
          this.style.left;
          return val;
        </setter>
      </property>

Comment 14

[Blake Ross]

That <setter> doesn't seem to do anything. It should be |return this.style.left
= val;|

Comment 15

[Blake Ross]

Actually, judging from Hixie's comment, you shouldn't be setting
this.style.left.  I haven't actually looked at the code as a whole, but the
setter as you pasted it doesn't do anything.

Comment 16

[Johnny Stenback (:jst)]

The more I think about this the more I think we should *not* do this. This has
been talked about before, and every time we've had this discussion we've decided
to not do this. If we put this in "the internal client" then I don't think we
(Netscape/AOL) has an option of not putting this in all future releases, and I
*really* don't want to make that decision with this short notice.

This change will pollute the document namespace on all quirk-mode pages we load
with the id's of all div's that have an id (or just absolutely positioned ones,
ideally, as I mentioned to doron in person), and it'll do so unconditionally.
Did 4x do that? I doubt it. I.e. what if I have a <div id="title"
style="position: absolute;">, what's then document.title? And so on.

This will most likely affect the future of Mozilla as well, or Netscape/AOL will
need to branch functionality wise, and I don't think that's a world we'd like
either.

To be honest, this scares me, a lot. The long-term consequences of this change
are unknown at this point, and I really think flipping this switch today w/o
putting more thought, testing, and discussion into it is a bad idea.

Not to mention the cost of a XBL binding on every div that has an id attribute
on most of the webpages out there.

Comment 17

[Johnny Stenback (:jst)]

Oh, and since people will, and already did, draw paralells with the marquee
support that was added to Mozilla lately, I want people to know that there's a
significant difference between doing this and adding support for marquee. The
marquee support that was added was completely non-destructive to web developers
and existing content. It harms nothing and changes nothing unless you use
marquee, in which case it does only good. This change, however, has the
potential of polluting the document namespace and can thus break things on
webpages that were designed w/o with this in mind. Sure, those pages would
probably have been broken in similar ways in 4x, but nevertheless, this breaks
things, our marquee support doesn't.

Comment 18

[Doron Rosenberg (IBM)]

I admit we are doing this at too short of a timeframe, and since jst doesn't
think its how we should do it, stoping and removing date.

Comment 19

[Hixie (not reading bugmail)]

I defer to jst. If he thinks this is a bad idea then I am more than happy for us
to WONTFIX the whole mess. As I said above, we have much more important (IMHO)
fish to fry. This kind of patch should only be taken if it is risk free.

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

As a 1.0 driver, I agree with jst and some of the other comments made by people
on #drivers.  Too late with too little time for either baking _or_ consideration
and argument for a significant about-face.  It can _always_ be taken on a local
tree by someone who wants it.  In general, compatibility is good - but we get
some disturbing side-effects as jst mentions, and there may be more; we haven't
had time to really think about it or test it on trunk.

Comment 21

[Frank Tang]

doron: althoght we decide not to take this fix, can you summarizied in a
document what is the issues need to be address to make mozilla backward
compatable with 4.x ?
I agree this approach may not be the righ one, but I think the NEED is still
there and won't change for at least 5 years. 

Comment 22

[Brendan Eich [:brendan]]

Mozilla should not try to be compatible with all of the 4.x proprietary DOM,
obviously (we've held off emulating layers, and will continue to do so, I hope).
 From comment #2, it seems sites are abandoning the old techniques (19) perhaps
as fast as sticking with the technique this bug's patch supplies (23).  Half
(42) of the sample mentioned in that comment still have problems even with this
bug's patch.  Are we supposed to extend the patch to make those sites work? 
Where does this end?

Has anyone even tried evangelism?  Cc'ing bclary.  I think evangelism is the way
to go here, especially if those 19 of 88 sites that used floating images no
longer do because the sites' authors noticed problems and removed the floating
images (meaning divs, I guess).  If this problem is going away, and can be made
to go away faster by evangelizing site authors on the merits of the standard DOM
(which works in most of the popular browsers, these days), everyone wins.

/be

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Did Netscape 4.x even do this, or is it just MSIE?

Comment 24

[Johnny Stenback (:jst)]

Yes, 4x did this for layers and for positioned div's (which were treated as layers).

Comment 25

[bobj]

> From comment #2, it seems sites are abandoning the old techniques (19)

We cannot draw that conclusion.  Many of these sites use the floating divs
for ads.  If they have not sold ads for that day, then they will not be
there.  When they sell ads, they may be using this technique.

Comment 26

[Doron Rosenberg (IBM)]

>Has anyone even tried evangelism?  Cc'ing bclary.  I think evangelism is the way
>to go here, especially if those 19 of 88 sites that used floating images no
>longer do because the sites' authors noticed problems and removed the floating
>images (meaning divs, I guess).  If this problem is going away, and can be made
>to go away faster by evangelizing site authors on the merits of the standard DOM
>(which works in most of the popular browsers, these days), everyone win

Indeed, evangelism is the way to go, but its a slow process.  This started when
I noticed that many sites (in china) use the same 4.x way to refer to an object,
and wrote some xbl to see if supporting it would fix these sites, which it did.
 It also had the side effect of fixing old dreamweaver code, which fixed a few
of european top sites.

Comment 27

[Brendan Eich [:brendan]]

bobj: you're right, I wrote "it seems" because there could be other reasons. 
Still, if we're not taking this patch, can we evangelize harder?  Evangelism is
a slow process, but lately we've had a lot of wins from the efforts of the
Netscape tech. evangelism team.  Have they even had a chance to try with these
China sites (or with the European dreamweaver-based sites)?

/be

Comment 28

[bobj]

Yes, I think everyone agrees that this approach is not good. We will rely upon
evangelism for now.  Doron has solutions for the floating divs used by most
of the top China sites.  But the real challenge, is to get the sites to apply
the solutions...

Comment 29

[Mathieu Pillard [:mat]]

btw, just a quite note about the side effects of this patch :
Ine ns4, when a form was inside a layer, to get a reference to it was :
document.layername.document.forname, layername being an id and formname, a name.
and the problem is, some sites actually use the same string for formname and
layername. they do something like this :
if (document.layers)
  blah = document.layername.document.forname
else
  blah = document.formname

now, with this patch, what will happen to us ? will 'blah' be a reference to the
form, or to the layer ? IE doesnt have this problem since it doesnt use
document.elementID ... NS4... well, I have no idea how NS4 behaved exactly :]
this also happens with others collections such as document.images,
document.applets, etc.

Comment 30

[Doron Rosenberg (IBM)]

inv

Comment 31

[timeless]

afaik this was superceded by bug 170375