Skip to content

Commit

Permalink
Make sure EMBED elements are extended manually in IE when inheritance…
Browse files Browse the repository at this point in the history 
… defficiency is detected. Avoid APPLET elements creation in that test, since it triggers warning in clients without Java. [#668 state:resolved][#681 state:resolved] (Nick Stakenburg, orv, Andrew Dupont, kangax)
  • Loading branch information
Juriy Zaytsev committed May 19, 2009
1 parent 6d9f8b8 commit 9020365
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 19 deletions.
24 changes: 10 additions & 14 deletions src/dom/dom.js
View file
Expand Up @@ -1626,25 +1626,21 @@ Element.extend = (function() {
}

var HTMLOBJECTELEMENT_PROTOTYPE_BUGGY = checkDeficiency('object');
var HTMLAPPLETELEMENT_PROTOTYPE_BUGGY = checkDeficiency('applet');

if (Prototype.BrowserFeatures.SpecificElementExtensions) {
// IE8 has a bug with `HTMLObjectElement` and `HTMLAppletElement` objects
// IE8 has a bug with `HTMLObjectElement`, `HTMLAppletElement` and `HTMLEmbedElement` objects
// (apparently implementing them as ActiveX/COM objects)
// not being able to "inherit" from `Element.prototype`
// or a specific prototype - `HTMLObjectElement.prototype`, `HTMLAppletElement.prototype`
if (HTMLOBJECTELEMENT_PROTOTYPE_BUGGY &&
HTMLAPPLETELEMENT_PROTOTYPE_BUGGY) {
// or a more specific - `HTMLObjectElement.prototype`, `HTMLAppletElement.prototype`, `HTMLEmbedElement.prototype`
// We only test for defficient OBJECT and assume that both - EMBED and APPLET are affected as well,
// since creating an APPLET element in IE installations without Java triggers warning popup, which we try to avoid
if (HTMLOBJECTELEMENT_PROTOTYPE_BUGGY) {
return function(element) {
if (element && element.tagName) {
var tagName = element.tagName.toUpperCase();
if (tagName === 'OBJECT' || tagName === 'APPLET') {
var t;
if (element && (t = element.tagName)) {

This comment has been minimized.

Sign in to view

Copy link
@tobie

tobie May 19, 2009

I know it's a pain to write this kind of construct otherwise... but I'd vote for avoiding those whenever possible (except in while loops maybe, where they kind of make sense).
if (/^(?:object|applet|embed)$/i.test(t)) {
extendElementWith(element, Element.Methods);
if (tagName === 'OBJECT') {
extendElementWith(element, Element.Methods.ByTag.OBJECT)
}
else if (tagName === 'APPLET') {
extendElementWith(element, Element.Methods.ByTag.APPLET)
}
extendElementWith(element, Element.Methods.ByTag[t.toUpperCase()]);
}
}
return element;
Expand Down
14 changes: 9 additions & 5 deletions test/unit/dom_test.js
View file
Expand Up @@ -641,6 +641,11 @@ new Test.Unit.Runner({
},

testElementExtend: function() {
function testTag(tagName) {
var element = document.createElement(tagName);
this.assertEqual(element, Element.extend(element));
this.assertRespondsTo('show', element);
}
var element = $('element_extend_test');
this.assertRespondsTo('show', element);

Expand All @@ -654,11 +659,10 @@ new Test.Unit.Runner({
'script select small span strong style sub sup '+
'table tbody td textarea tfoot th thead tr tt ul var');

XHTML_TAGS.each(function(tag) {
var element = document.createElement(tag);
this.assertEqual(element, Element.extend(element));
this.assertRespondsTo('show', element);
}, this);
XHTML_TAGS.each(testTag, this);

// EMBED is deprecated, but is widely used
testTag.call(this, 'embed');

[null,'','a','aa'].each(function(content) {
var textnode = document.createTextNode(content);
Expand Down

6 comments on commit 9020365

@tobie
Copy link

@tobie tobie commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two other comments:

I can't see any caching mechanisme for either of those 3 special elements. Is there a reason for this ?

What happened to simulated methods (I know hasAttribute was fixed in IE 8 but I really dislike the idea of keeping a half baked mechanism around)

@tobie
Copy link

@tobie tobie commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's a shame you can't preview comments at the bottom of the page :/

Also, wished MArkdown didn't use # for marking titles when Textile uses it to create numbered lists :|

@kangax
Copy link
Owner

@kangax kangax commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn. If we remove t = element.tagName, the whole thing becomes ugly :/ Good call about caching, I forgot about it. What about simulated? Element.Methods.Simulated is still in the source.

Here's an updated assignment-in-an-if-expression-less version:

if (HTMLOBJECTELEMENT_PROTOTYPE_BUGGY) {
  return function(element) {
    if (element && typeof element._extendedByPrototype == 'undefined') {
      var t = element.tagName;
      if (t) {
        if (/^(?:object|applet|embed)$/i.test(t)) {
          extendElementWith(element, Element.Methods);
          extendElementWith(element, Element.Methods.ByTag[t.toUpperCase()]);
        }
      }
    }
    return element;
  }
}

@kangax
Copy link
Owner

@kangax kangax commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those 2 if don't make much sense, of course:

if (HTMLOBJECTELEMENT_PROTOTYPE_BUGGY) {
  return function(element) {
    if (element && typeof element._extendedByPrototype == 'undefined') {
      var t = element.tagName;
      if (t && /^(?:object|applet|embed)$/i.test(t)) {
        extendElementWith(element, Element.Methods);
        extendElementWith(element, Element.Methods.ByTag[t.toUpperCase()]);
      }
    }
    return element;
  }
}

@tobie
Copy link

@tobie tobie commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about simply using an early return like it's done in the original function ?

Also, why are you guarding against null or empty tagNames, when is that an issue?

Regarding Simulated methods, would those be added to object, applet or embed tags? I'm not sure from the code I see above.

@kangax
Copy link
Owner

@kangax kangax commented on 9020365 May 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about simply using an early return like it's done in the original function?

Does it really matter?

Also, why are you guarding against null or empty tagNames, when is that an issue?

When a non-element is passed into Element.extend (e.g. document or window)

Regarding Simulated methods, would those be added to object, applet or embed tags? I'm not sure from the code I see above.

Just checked - they are not added. It "works" because hasAttribute (the only method in Simulated) is implemented natively in IE8. This is clearly an oversight, since our docs, at least, guarantee that all of Simulated methods exist on extended elements.

Please sign in to comment.