#763 ✓incomplete
Carl Woodward

Safari 4 - problem with element.getElementsBySelector()

Reported by Carl Woodward | August 10th, 2009 @ 03:39 AM

There is a really wierd assign in findElements which breaks safari 4.

root.id = oldId;

The declaration of oldId doesn't look to be scoped properly either.

When I commend out that line everything works perfectly.

I'd appreciate any thoughts.

    // Original version.
findElements: function(root) {
    root = root || document;
    var e = this.expression, results;
 
    switch (this.mode) {
      case 'selectorsAPI':
        // querySelectorAll queries document-wide, then filters to descendants
        // of the context element. That's not what we want.
        // Add an explicit context to the selector if necessary.
        if (root !== document) {
          var oldId = root.id, id = $(root).identify();
          e = "#" + id + " " + e;
        }
 
        results = $A(root.querySelectorAll(e)).map(Element.extend);
        root.id = oldId;
 
        return results;
      case 'xpath':
        return document._getElementsByXPath(this.xpath, root);
      default:
       return this.matcher(root);
    }
  },
 
 
// Changed version to work in safari.
findElements: function(root) {
    root = root || document;
    var e = this.expression, results;
 
    switch (this.mode) {
      case 'selectorsAPI':
        // querySelectorAll queries document-wide, then filters to descendants
        // of the context element. That's not what we want.
        // Add an explicit context to the selector if necessary.
        if (root !== document) {
          //var oldId = root.id,
          var id = $(root).identify();
          e = "#" + id + " " + e;
        }
        
        results = $A(root.querySelectorAll(e)).map(Element.extend);
        // root.id = oldId;
 
        return results;
      case 'xpath':
        return document._getElementsByXPath(this.xpath, root);
      default:
       return this.matcher(root);
    }
  },

Comments and changes to this ticket

  • Tobie Langel

    Tobie Langel August 10th, 2009 @ 05:15 PM

    • State changed from “new” to “incomplete”

    Thanks for your ticket. Unfortunately we aren't able to replicate the problem you've described. Could you please reopen this ticket with a failing test case attached?

    Please also note that there is no scope issue here: IN JavaScript, blocks don't have a scope of their own only functions do.

  • Diego Perini

    Diego Perini August 15th, 2009 @ 01:18 AM

    • Assigned user set to “Tobie Langel”

    Don't know if this has something to do with the above problem but it seems to me that with current code there is the possibility that "root.id" is set to "undefined" instead of an empty string ('').

    The case shows up when the "root" parameter is not passed in or when it is a "document" reference.

    To fix this please declare "oldid" to initially be an empty string like this:

    var e = this.expression, oldid = '', results;
    

    and remove the "var" keyword inside the conditional block.

    Maybe the OP could try this fix and tell us if it cures the described problem.

  • Diego Perini

    Diego Perini August 15th, 2009 @ 01:25 AM

    Well re-looking at this it may also be that in case "root === document" it was indeed the intention of the writer to set "document.id = undefined", but at this point I feel this should just be skipped.

    It is just something I guessed by reading the ticket, not sure yet it is a bug or it is related to the OP.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Pages