I'm going to start working on version 2 trying to incorporate all the
feedback so far.


http://codereview.appspot.com/44041/diff/1/3
File user/src/com/google/gwt/dom/client/DOMImplSafari.java (right):

http://codereview.appspot.com/44041/diff/1/3#newcode173
Line 173: public native NodeList<Element> getElementsByXPath(Element
parent, String xPath) /*-{
On 2009/05/14 18:50:42, cromwellian wrote:
> Where is this _getElementsByXPath function? Is this code copied from
> prototype.js? Also, if this is a function you've inserted on document,
you
> should probably use $doc

Sorry - my bad.  Didn't read the code from the
http://webkit.org/blog/153/webkit-gets-native-getelementsbyclassname/
carefully enough.  This function can be removed - the one provided in
DOMImplStandard is the one that should be used.

http://codereview.appspot.com/44041/diff/1/4
File user/src/com/google/gwt/dom/client/DOMImplStandard.java (right):

http://codereview.appspot.com/44041/diff/1/4#newcode159
Line 159: document.getElementsByClassName =
Element.prototype.getElementsByClassName = xpathImpl;
On 2009/05/14 18:50:42, cromwellian wrote:
> This code by default will run in an IFRAME that is different than the
host HTML
> page, so I'm not sure this is doing what you want, since you've
overriding
> methods on the document object and Element.prototype of the IFRAME
which loaded
> the GWT code, not the one that has the elements you want to search.
$doc refers
> to the document object of the host page.


You are correct.  I should set them for the parent page, but please look
at the comment below for my proposed correction (this portion of code
remains unchanged).

http://codereview.appspot.com/44041/diff/1/4#newcode169
Line 169: if (!document.getElementsByClassName) {
On 2009/05/14 18:50:42, cromwellian wrote:
> The checks on document here are ok because another module or included
JS library
> might stomp on $doc.getElementsByClassName, so checking the private
one in the
> IFRAME is better. However, if you truly want to check if a native
implementation
> exists, you can't just test for the existence of the function.

Actually, AFAIK, that's exactly the test to use to determine if the
native implementation exists (or in the more general case, if any
implementation, native or otherwise, of a JS function exists).

As for the stomping, I agree, but please comment on by reasoning in the
added snippet below.

http://codereview.appspot.com/44041/diff/1/4#newcode175
Line 175: }
I think I'm missing this code here to make it work:
   if (!$doc.getElementsByClassName) {
     $doc.getElementsByClassName = document.getElementsByClassName;
   }

   if (!$wnd.Element.prototype.getElementsByClassName) {
     $wnd.Element.prototype.getElementsByClassName =
Element.prototype.getElementsByClassName;
   }

In a little non-GWT snippet I wrote,
window.parent.Element.prototype.getElementsByClassName seemed to work
from within the iFrame on FF 3.5 in Linux.  Would this be an appropriate
generic solution?

Also, to detect stomping, I think, if the functions are already defined
in $doc but not document, we should give the developer some kind of
warning message that there might be a conflict with one of the libraries
he/she uses.  What do you think?  A call to GWT.log should probably
suffice.

http://codereview.appspot.com/44041

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to