Oops - missed some compile errors in the JSNI code. This is meant to be
applied over top of the previous patches. I've verified that if
getElementsByClassName is not used then no code related to it appears in the
output (compiled the GWT samples & grepped for getElementsByClassName).
I still need to test whether or not the implementations work & write unit
tests.
Just for easy reference for myself, [].slice.call(nodeList, 0); is the code
to convert a nodelist into an array.
On Sun, Apr 19, 2009 at 5:47 AM, Vitali Lovich <[email protected]> wrote:
> Here's my second attempt at the patch (2 of 2 is just a documentation
> adjustment).
>
> One caveat I forgot to mention is that there is slightly different
> cross-platform behaviour right now. The native implementations return
> NodeLists which are live whereas the Javascript implementations simply
> return arrays.
>
> The question is should I change it so that a JsArray is always returned
> (i.e. copy the node list into an array) so that semantics are constant
> across browsers or leave as is for performance with maybe a caveat that the
> user should be careful when using with FF3, Safari 3.1 or Opera 9.5 & copy
> into an array if they need array semantics?
>
> Changes from first patch:
> getElementsByClassName should now display the HTML5 behaviour in quirks
> mode if the browser supports the native implementation or doesn't provide
> XPath (IE or really really old versions of FF, Safari & friends). I'm not
> sure how to get case insensitive string matching with XPath (so quirks mode
> won't work according to the HTML5 spec in FF2, Safari 3, Opera 9.4 or
> earlier). Is it even possible to do, or should I just fall back to using
> the DOM implementation in quirks mode & make a note that performance could
> be improved drastically if quirks mode is turned off on browsers that
> support XPath? Or just leave as is - it'll certainly have the same
> behaviour as all the other JS libraries out there.
>
> Only DOMImpl, DOMImplStandard, & DOMImplSafari now require code changes.
>
> DOMImpl: contains a native implementation using DOM. By default this gets
> called.
> DOMImplStandard now defines getElementsByXPath. The fastest approach is
> assigned* to getElementsByClassName in the Element prototype & document.
> overrides getElementsByClassName
> DOMImplSafari: getElementsByXPath calls the native one supplied by
> Safari. I'm assuming that document._getElementsByXPath & document.evaluate
> came in with the same version of Safari (the one that implemented
> XPath). If that's not the case, then I'll have to rework the code a bit.
> Document: added isBackCompat(). Could use !isCSS1Compat() for now, but
> this ensures no code change required if other compat modes are added in the
> future ('cause !quirks is all we care about).
>
> *I'm assuming that the way I wrote it ensures that if the developer doesn't
> use getElementsByClassName, then there's 0-cost: the function is never
> registered because the GWT compiler will recognize the dead code.
>
> If it is used, then there's only the overhead of performing the
> implementation selection once on startup.
>
> Thank you,
> Vitali
>
>
> On 4/18/09, Vitali Lovich <[email protected]> wrote:
> > Also, what's the difference between DOMImplMozilla & DOMImplMozillaOld
> > in terms of Firefox versions?
> >
> > And what versions of each browser are supported? Are all versions of
> > Safari, FF, & Opera supported as much as possible? Is there a minimum
> > version for any of them?
> >
> >
> > On Sat, Apr 18, 2009 at 4:05 AM, Vitali Lovich <[email protected]>
> wrote:
> > > If I put the check in a static block, then the check is only done once
> > > on startup, right? Is this acceptable, or is it frowned upon because
> > > it adds a slight penalty to the startup even if the developer never
> > > uses the function.
> > >
> > > Something like,
> > >
> > > static {
> > > ensureGetElementsByClassNameRegistered();
> > > }
> > >
> > > private static native void ensureGetElementsByClassNameRegistered() {
> > > if (!document.getElementsByClassName) {
> > > document.getElementsByClassName = function(element, class) {
> > > // etc
> > > }
> > > }
> > > }
> > >
> > > On Sat, Apr 18, 2009 at 3:54 AM, Ray Cromwell <[email protected]>
> wrote:
> > >>
> > >> You don't want to create a separate property just for native
> > >> getElementsByClassName() as that would almost double the number of
> > >> permutations (except IE where is it known not to exist) What you
> > >> could provide is 3 implementations. IE gets raw DOM version,
> > >> Safari/Firefox/Opera get a version that checks
> > >> if(document.getElementsByClassName) exists, and if not, falls back to
> > >> document.evaluate.
> > >>
> > >> -Ray
> > >>
> > >>
> > >> On Fri, Apr 17, 2009 at 11:03 PM, Vitali Lovich <[email protected]>
> wrote:
> > >>>
> > >>> Do you have any suggestions regarding how to do the proper deferred
> > >>> binding for the various versions? Should I just put the typical JS
> > >>> check that regular libraries use? (i.e. if
> > >>> (!document.getElementsByClassName) { // provide fallback }. I
> thought
> > >>> the whole point of deferred binding was to get around that.
> > >>>
> > >>> What is the list of officially supported browsers? In particular
> the
> > >>> specific minimum version for each?
> > >>>
> > >>> I guess I'm going to have to install a virtual machine for this
> > >>> because Safari refuses to work in wine 1.0.19 - chrome is useable
> > >>> although no HTTPS sites.
> > >>>
> > >>> Is there a guide somewhere on writing & integrating the tests into
> > >>> GWT, or just look at existing ones in the source.
> > >>>
> > >>> On Sat, Apr 18, 2009 at 1:49 AM, Miroslav Pokorny
> > >>> <[email protected]> wrote:
> > >>>>
> > >>>> Provide some tests and make sure it works on all officially
> supported
> > >>>> browsers even if the implementation is suboptimal. There are many
> js
> > >>>> libs out there so it's not hard to find the best way to do it fir
> the
> > >>>> various browsers...
> > >>>>
> > >>>> On 18/04/2009, at 12:39 PM, Vitali Lovich <[email protected]>
> wrote:
> > >>>>
> > >>>>> Just my first attempt at adding some initial support for
> > >>>>> getElementsByClassName. This is by no means tested or
> > >>>>>
> > >>>>> Newer browsers support getElementsByClassName. This is my attempt
> > >>>>> at a patch.
> > >>>>>
> > >>>>> Caveats:
> > >>>>> There's a "bug" in that quirks mode doesn't behave properly (I
> don't
> > >>>>> know how to detect it in the deferred binding) for IE & Firefox2 &
> > >>>>> earlier. According to the HTML5 spec, it's supposed to be case
> > >>>>> insensitive in quirks mode. However, this "bug" behaves the exact
> > >>>>> same way as all the javascript wrappers do (since I based the
> XPath &
> > >>>>> RegExp code paths roughly around the implementations floating out
> > >>>>> there).
> > >>>>>
> > >>>>> I'm also not sure if I added the support correctly for Firefox.
> > >>>>> getElementsByClassName only came in with FF3 & requires the xpath
> > >>>>> workaround for FF2 & earlier (I believe I read xpath was
> introduced
> > >>>>> with FF 1.5). I placed the xpath code in DOMImplMozillaOld for
> now
> > >>>>> (not sure whether or not gecko1_8 represents FF2).
> > >>>>>
> > >>>>> There's also no mention of the versions of Safari & Opera that are
> > >>>>> supported by GWT, so they're defaulting to using the native
> version
> > >>>>> (which will not work if they don't provide it).
> > >>>>>
> > >>>>> Safari in particular got it with version 3.1. if earlier versions
> are
> > >>>>> supported, it's not too much work. Simply grab the expression
> built
> > >>>>> in the FF xpath & pass it to document._getElementsByXPath(query,
> > >>>>> parent) for Safari versions that have xpath (not sure which those
> > >>>>> are). Not sure how to distinguish Safari pre3.1 & post3.1 in
> deferred
> > >>>>> binding.
> > >>>>>
> > >>>>> Opera got it with 9.5. Not sure when xPath support got into Opera
> > >>>>> (should be a straightfoward copy of the mozilla code). Not sure
> how
> > >>>>> to distinguish the two in the deferred binding.
> > >>>>>
> > >>>>> Feedback would be appreciated.
> > >>>>>
> > >>>>> Should this go as an enhancement request in the issue tracker?
> > >>>>>
> > >>>>> >
> > >>>>> diff --git a/user/src/com/google/gwt/dom/client/DOMImpl.java
> b/user/
> > >>>>> src/com/google/gwt/dom/client/DOMImpl.java index 1d7e39b..581764b
> > >>>>> 100644 --- a/user/src/com/google/gwt/dom/client/DOMImpl.java +++
> b/
> > >>>>> user/src/com/google/gwt/dom/client/DOMImpl.java @@ -176,6 +176,8
> @@
> > >>>>> abstract class DOMImpl { return 0; }-*/; + public abstract
> NodeList
> > >>>>> getElementsByClassName(Element parent, String className); + public
> > >>>>> native Element getFirstChildElement(Element elem) /*-{ var child =
> > >>>>> elem.firstChild; while (child && child.nodeType != 1) diff --git
> a/
> > >>>>> user/src/com/google/gwt/dom/client/DOMImplIE6.java b/user/src/com/
> > >>>>> google/gwt/dom/client/DOMImplIE6.java index 04c205a..8c24a6c
> 100644
> > >>>>> --- a/user/src/com/google/gwt/dom/client/DOMImplIE6.java +++
> b/user/
> > >>>>> src/com/google/gwt/dom/client/DOMImplIE6.java @@ -15,6 +15,8 @@ */
> > >>>>> package com.google.gwt.dom.client; +import
> > >>>>> com.google.gwt.core.client.JsArray; + /** * Internet Explorer 6
> > >>>>> implementation of * {...@link
> com.google.gwt.user.client.impl.DOMImpl}.
> > >>>>> @@ -184,6 +186,31 @@ class DOMImplIE6 extends DOMImpl { }
> @Override
> > >>>>> + public NodeList getElementsByClassName(Element parent, String
> > >>>>> className) { + // TODO: detect quirks mode so that the regexps do
> a
> > >>>>> case insensitive match as per the + // spec. when quirks mode
> > >>>>> detected, pass "i" as the flags parameter + return
> > >>>>> getElementsByClassName(parent, className.split(" "), "").cast(); +
> }
> > >>>>> + + private native JsArray getElementsByClassName(Element parent,
> > >>>>> String [] classes, String flags) /*-{ + var i, j, e, c, elements =
> > >>>>> parent.all || parent.getElementsByTagName("*"); + var regexps =
> [],
> > >>>>> result = []; + for (i = classes.length - 1; i >= 0; i--) { + c =
> > >>>>> classes[i].replace(/^\s+|\s+$/g, ""); + if (c.length) +
> > >>>>> regexps.push(new RegExp(new Array("(?:^|\\s)", c, "(?:\\s|
> > >>>>> $)").join(""), flags); + } + if (regexps.length) { + for (i = 0, c
> =
> > >>>>> elements.length; i < c; i++) { + e = elements[i]; + for (j =
> > >>>>> regexps.length - 1; j >= 0 && regexps[j].test(e); j--) {} + j ==
> -1
> > >>>>> && result.push(e); + } + } + return result; + }-*/; + + @Override
> > >>>>> public native String getInnerText(Element elem) /*-{ return
> > >>>>> elem.innerText; }-*/; diff --git a/user/src/com/google/gwt/dom/
> > >>>>> client/DOMImplMozillaOld.java
> b/user/src/com/google/gwt/dom/client/
> > >>>>> DOMImplMozillaOld.java index ecf52fb..5fd7312 100644 ---
> a/user/src/
> > >>>>> com/google/gwt/dom/client/DOMImplMozillaOld.java +++
> b/user/src/com/
> > >>>>> google/gwt/dom/client/DOMImplMozillaOld.java @@ -15,6 +15,8 @@ */
> > >>>>> package com.google.gwt.dom.client; +import
> > >>>>> com.google.gwt.core.client.JsArray; + /** * DOM implementation
> > >>>>> differences for older version of Mozilla (mostly the * hosted mode
> > >>>>> browser on linux). The main difference is due to changes in @@
> > >>>>> -114,4 +116,23 @@ package com.google.gwt.dom.client; return top +
> > >>>>> viewport.scrollTop; }-*/; + + @Override + public NodeList
> > >>>>> getElementsByClassName(Element parent, String className) { + //
> the
> > >>>>> cast should be safe since both it & JsArray access the underlying
> > >>>>> object the same way + // and NodeList is immutable (constraints
> > >>>>> added, not removed) + return getElementsByClassName(parent,
> > >>>>> className.split(" ")).cast(); + } + + private native JsArray
> > >>>>> getElementsByClassName(Element parent, String [] classes) /*-{ +
> var
> > >>>>> result, i, xpathResult, classExpr = ".//*"; + for (i =
> > >>>>> classes.length - 1; i >= 0; i--) { + classExpr +=
> > >>>>> "[contains(concat(' ', @class, ' '), ' " + classes[i] + " ')]"; +
> }
> > >>>>> + xpathResult = document.evaluate(classExpr, parent, null, 0,
> null);
> > >>>>> + while ((i = xpathResult.iterateNext())) { + result.push(i); + }
> +
> > >>>>> return result; + }-*/; } diff --git a/user/src/com/google/gwt/dom/
> > >>>>> client/DOMImplStandard.java b/user/src/com/google/gwt/dom/client/
> > >>>>> DOMImplStandard.java index 90d2e89..be29fe1 100644 --- a/user/src/
> > >>>>> com/google/gwt/dom/client/DOMImplStandard.java +++ b/user/src/com/
> > >>>>> google/gwt/dom/client/DOMImplStandard.java @@ -114,6 +114,11 @@
> > >>>>> abstract class DOMImplStandard extends DOMImpl { }-*/; @Override +
> > >>>>> public native NodeList getElementsByClassName(Element parent,
> String
> > >>>>> className) /*-{ + return parent.getElementsByClassName(className);
> > >>>>> + }-*/; + + @Override public native boolean isOrHasChild(Element
> > >>>>> parent, Element child) /*-{ return parent.contains(child); }-*/;
> > >>>>> diff --git a/user/src/com/google/gwt/dom/client/Document.java
> b/user/
> > >>>>> src/com/google/gwt/dom/client/Document.java index 209b1b9..23d6daf
> > >>>>> 100644 --- a/user/src/com/google/gwt/dom/client/Document.java +++
> b/
> > >>>>> user/src/com/google/gwt/dom/client/Document.java @@ -1125,6
> +1125,18
> > >>>>> @@ public class Document extends Node { }-*/; /** + * Returns a
> > >>>>> {...@link NodeList} of all the {...@link Element Elements} with a
> > given
> +
> > >>>>> * class name in the order in which they are encountered in a
> > >>>>> preorder traversal + * of the document tree. + * + * @param
> > >>>>> className the space-separated name of the classes to match on + *
> > >>>>> @return a list containing all the matched elements. + */ + public
> > >>>>> final NodeList getElementsByClassName(String className) { + return
> > >>>>> DOMImpl.impl.getElementsByClassName(this.cast(), className); + } +
> > >>>>> + /** * Returns the {...@link Element} whose id is given by
> elementId.
> > >>>>> If no such * element exists, returns null. Behavior is not defined
> > >>>>> if more than one * element has this id. diff --git a/user/src/com/
> > >>>>> google/gwt/dom/client/Element.java b/user/src/com/google/gwt/dom/
> > >>>>> client/Element.java index a0bf9f2..d64b83e 100644 ---
> a/user/src/com/
> > >>>>> google/gwt/dom/client/Element.java +++
> b/user/src/com/google/gwt/dom/
> > >>>>> client/Element.java @@ -149,6 +149,18 @@ public class Element
> > >>>>> extends Node { }-*/; /** + * Returns a {...@link NodeList} of all the
> > >>>>> {...@link Element Elements} with a given + * class name in the order
> in
> > >>>>> which they are encountered in a preorder traversal + * of this
> > >>>>> elements children. + * + * @param className the space-separated
> > >>>>> names of the classes to match on + * @return a list containing all
> > >>>>> the matched elements. + */ + public final NodeList
> > >>>>> getElementsByClassName(String className) { + return
> > >>>>> DOMImpl.impl.getElementsByClassName(this, className); + } + + /**
> *
> > >>>>> Returns a NodeList of all descendant Elements with a given tag
> name,
> > >>>>> in the * order in which they are encountered in a preorder
> traversal
> > >>>>> of this Element * tree.
> > >>>>
> > >>>> >
> > >>>>
> > >>>
> > >>> >
> > >>>
> > >>
> > >> > > >>
> > >>
> > >
> >
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
diff --git a/user/src/com/google/gwt/dom/client/DOMImpl.java b/user/src/com/google/gwt/dom/client/DOMImpl.java
index 3eae8ba..bf6dd6c 100644
--- a/user/src/com/google/gwt/dom/client/DOMImpl.java
+++ b/user/src/com/google/gwt/dom/client/DOMImpl.java
@@ -185,7 +185,7 @@ abstract class DOMImpl {
for (i = classes.length - 1; i >= 0; i--) {
c = classes[i].replace(/^\s+|\s+$/g, "");
if (c.length) {
- regexps.push(new RegExp(new Array("(?:^|\\s)", c, "(?:\\s|$)").join(""), inQuirksMode ? "i", "");
+ regexps.push(new RegExp(new Array("(?:^|\\s)", c, "(?:\\s|$)").join(""), inQuirksMode ? "i" : ""));
}
}
if (regexps.length) {
diff --git a/user/src/com/google/gwt/dom/client/DOMImplStandard.java b/user/src/com/google/gwt/dom/client/DOMImplStandard.java
index 237b0e0..398c8d0 100644
--- a/user/src/com/google/gwt/dom/client/DOMImplStandard.java
+++ b/user/src/com/google/gwt/dom/client/DOMImplStandard.java
@@ -154,23 +154,23 @@ abstract class DOMImplStandard extends DOMImpl {
for (i = classes.length - 1; i >= 0; i--) {
classExpr += "[contains(concat(' ', @class, ' '), ' " + classes[i] + " ')]";
}
- return @com.google.gwt.dom.client.DOMImpl.impl.getElementsByXPath(Lcom/google/gwt/dom/client/ElementLjava/lang/String)(this, classExpr);
+ return @com.google.gwt.dom.client.DOMImpl::impl.getElementsByXPath(this, classExpr);
}
document.getElementsByClassName = Element.prototype.getElementsByClassName = xpathImpl;
}-*/;
protected native void registerDOM(boolean inQuirksMode) /*-{
document.getElementsByClassName = Element.prototype.getElementsByClassName = function(className) {
- return @com.google.gwt.dom.client.DOMImpl.getElementsByClassNameDOMImpl(this, className);
+ return @com.google.gwt.dom.client.DOMImpl::impl.getElementsByClassNameDOMImpl(this, className);
}
}-*/;
protected native void register(boolean inQuirksMode) /*-{
if (!document.getElementsByClassName) {
- th...@com.google.gwt.dom.client.domimplstandard.elementsbyclassname::registerXPath(inQuirksMode);
if (document.evaluate) {
+ th...@com.google.gwt.dom.client.domimplstandard.elementsbyclassname::registerXPath(Z)(inQuirksMode);
} else {
- th...@com.google.gwt.dom.client.domimplstandard.elementsbyclassname::registerDOM(inQuirksMode);
+ th...@com.google.gwt.dom.client.domimplstandard.elementsbyclassname::registerDOM(Z)(inQuirksMode);
}
}
}-*/;
