I signed the CLA. Should be OK for now since I haven't started my job yet.
I'm getting a HTTP SERVER 500 error creating the issue. I've tried both upload.py & the web interface. On Mon, Apr 20, 2009 at 10:58 AM, Joel Webber <[email protected]> wrote: > Vitali, > Thanks for taking this on. I've been meaning to do it for ages, but haven't > had time. I haven't had a chance to look over the patch in detail yet, but > the general approach sounds good. > > To answer your earlier question about DOMImplMozilla[Old], the "old" one > (not the best name, I know) maps to the user-agent "gecko", which is the > fallback for pre-gecko-1.8. Gecko 1.8 is basically Firefox 1.5 and 2.0, and > is also used to support later versions (e.g., Firefox 3 is Gecko 1.9). Older > versions of Gecko don't exist in any browser you're likely to encounter in > the wild, but is used by the hosted-mode browser on Linux. As soon as we > move to out-of-process hosted mode (GWT 2.0 in all likelihood), we'll be > able to ditch support for old Geckos (yay!). > > A couple of procedural things: > - Have you had a chance to sign a CLA yet? It's just a minor procedural > thing, but necessary. > (http://code.google.com/webtoolkit/makinggwtbetter.html#committers) > - Could I get you to create an issue to track this? > - Also, if you could upload this patch to > http://gwt-code-reviews.appspot.com/ it would make it easier for everyone > to view and discuss. > (Make sure to add [email protected] to > the CC list) > > Thanks again, > joel. > > On Sun, Apr 19, 2009 at 6:11 AM, Vitali Lovich <[email protected]> wrote: > >> 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 -~----------~----~----~----~------~----~------~--~---
