[gwt-contrib] RR: make PopupPanels stay on the screen, even in RTL situations
Hey Rajeev,
Would you review this patch for PopupPanel? It's a cleaned up version
of what we did yesterday. This fixes the odd behavior we noticed where
Popup Panels could not be placed near the right edge of the screen on
IE, and normalizes popups in RTL and LTR modes, so they can't be
dragged off the horizontal edges in either case.
Testing: we made sure popup panels worked as expected in Showcase on
all major browsers, both in Quirks and Standards mode. GWT tests all
still work.
Thanks!
--
Alex Rudnick
swe, gwt, atl
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Index: user/src/com/google/gwt/user/client/ui/PopupPanel.java
===
--- user/src/com/google/gwt/user/client/ui/PopupPanel.java (revision 3693)
+++ user/src/com/google/gwt/user/client/ui/PopupPanel.java (working copy)
@@ -575,11 +575,24 @@
public void setPopupPosition(int left, int top) {
// Keep the popup within the browser's client area, so that they can't get
// 'lost' and become impossible to interact with. Note that we don't attempt
-// to keep popups pegged to the bottom and right edges, as they will then
-// cause scrollbars to appear, so the user can't lose them.
-if (left 0) {
- left = 0;
+// to keep popups pegged to the bottom edge; dragging a popup off the bottom
+// will cause scrollbars to appear, so the user can't lose them.
+
+int popupWidth = getOffsetWidth();
+int popupRightEdge = left + popupWidth;
+int clientLeft = Document.get().getBodyOffsetLeft();
+int clientWidth = Window.getClientWidth() + clientLeft;
+
+if (popupRightEdge = clientWidth) {
+ left = clientWidth - popupWidth;
}
+
+// Not always 0. For example, on IE in RTL mode, there's a scrollbar
+// on the left-hand side.
+if (left clientLeft) {
+ left = clientLeft;
+}
+
if (top 0) {
top = 0;
}
[gwt-contrib] code review requested, shard TypeSerializerCreator createMethodMap
[fix bad GWTC address]
Please review the attached patch for trunk, which fixes a hosted mode crash
with a large set of serializable types.
On Linux hosted mode, a very large list of serializable types can cause a
crash inside Mozilla. This appears to be related to the size of the JSNI
function being evaluated to inject the code into the browser.
This patch adds a Java system property to control this behavior, and code to
split the createMethodMap if the numer of types exceeeds the specified
threshold. If no parameter is supplied, the output code is unchanged.
After we get some feedback on exactly what this threshold should be, we may
change it to a reasonable default. This parameter may also go away once
OOPHM is merged into trunk if it is no longer needed. So, this should be
considered a short-term patch.
The relevant generated code previously looked like this:
public static native JavaScriptObject createMethodMap() /*-{
return {
[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907: [
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
],
...
};
}-*/;
it now looks like this if the threshold is exceeded:
public static JavaScriptObject createMethodMap() {
JavaScriptObject map = JavaScriptObject.createObject();
createMethodMap_0(map);
createMethodMap_50(map);
...
return map;
}
public static native void createMethodMap_0(JavaScriptObject map) /*-{
map[[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907]=[
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
];
}-*/;
Note that the resulting code will be slightly larger and slower than before,
so this should only be used for Linux hosted mode where needed. Since the
default behavior is unchanged, if you do nothing there will be no impact on
the output.
--
John A. Tamplin
Software Engineer (GWT), Google
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Index: user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
===
--- user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java (revision 3687)
+++ user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java (working copy)
@@ -19,6 +19,7 @@
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.ext.GeneratorContext;
import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.core.ext.typeinfo.JClassType;
import com.google.gwt.core.ext.typeinfo.JMethod;
import com.google.gwt.core.ext.typeinfo.JParameterizedType;
@@ -32,6 +33,8 @@
import com.google.gwt.user.rebind.SourceWriter;
import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
/**
* This class generates a class with name 'typeSerializerClassName' that is able
@@ -52,6 +55,17 @@
+ SerializationStreamWriter streamWriter, Object instance, String typeSignature)
+ throws SerializationException;
+ /**
+ * Default number of types to split createMethodMap entries into. Zero means
+ * no sharding occurs.
+ */
+ private static final int DEFAULT_CREATEMETHODMAP_SHARD_SIZE = 0;
+
+ /**
+ * Java system property name to override the above.
+ */
+ private static final String GWT_CREATEMETHODMAP_SHARD_SIZE = gwt.typecreator.shard.size;
+
private final GeneratorContext context;
private final JType[] serializableTypes;
@@ -77,7 +91,7 @@
srcWriter = getSourceWriter(logger, context);
}
- public String realize(TreeLogger logger) {
+ public String realize(TreeLogger logger) throws UnableToCompleteException {
logger = logger.branch(TreeLogger.DEBUG,
Generating TypeSerializer for service interface '
+ getTypeSerializerClassName() + ', null);
@@ -92,7 +106,7 @@
writeCreateMethods();
-writeCreateMethodMapMethod();
+
[gwt-contrib] Re: code review requested, shard TypeSerializerCreator createMethodMap
Mostly LGTM. The only (marginally) substantive complaint is that it may
make sense to rework the property parsing around lines 285-293 so that it's
done once, statically, rather than re-parsing the String-int conversion for
every TypeSerializer we create, especially if (as I suggest as a nit) you
change to use the System.getProperty(String propname, String default)
formulation. Not that parsing a short string into int is any large amount
lot of work, but we may end up doing it often...
Nits
- Can we note, probably in the comments around lines 58-66, that the
shard size is measured in number of reachable types, not in e.g.
characters? (Yeah, I know characters would be hard to get at *a priori*,
but since our issue seems to be string length of the JSNI body, that's what
I expected size to be measured in.)
- Also mentioned above, line 285-286, why avoid the
System.getProperty(String prop, String default) formulation? (You'd have to
make the default a string, yes, but it saves you the if-null test.)
- I'll take your word for the style-correctness of the added space on
line 190.
On Tue, Sep 30, 2008 at 2:59 PM, John Tamplin [EMAIL PROTECTED] wrote:
[fix bad GWTC address]
Please review the attached patch for trunk, which fixes a hosted mode crash
with a large set of serializable types.
On Linux hosted mode, a very large list of serializable types can cause a
crash inside Mozilla. This appears to be related to the size of the JSNI
function being evaluated to inject the code into the browser.
This patch adds a Java system property to control this behavior, and code
to split the createMethodMap if the numer of types exceeeds the specified
threshold. If no parameter is supplied, the output code is unchanged.
After we get some feedback on exactly what this threshold should be, we may
change it to a reasonable default. This parameter may also go away once
OOPHM is merged into trunk if it is no longer needed. So, this should be
considered a short-term patch.
The relevant generated code previously looked like this:
public static native JavaScriptObject createMethodMap() /*-{
return {
[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907: [
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
],
...
};
}-*/;
it now looks like this if the threshold is exceeded:
public static JavaScriptObject createMethodMap() {
JavaScriptObject map = JavaScriptObject.createObject();
createMethodMap_0(map);
createMethodMap_50(map);
...
return map;
}
public static native void createMethodMap_0(JavaScriptObject map) /*-{
map[[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907]=[
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
@com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
];
}-*/;
Note that the resulting code will be slightly larger and slower than
before, so this should only be used for Linux hosted mode where needed.
Since the default behavior is unchanged, if you do nothing there will be no
impact on the output.
--
John A. Tamplin
Software Engineer (GWT), Google
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: code review requested, shard TypeSerializerCreator createMethodMap
On Tue, Sep 30, 2008 at 4:27 PM, John Tamplin [EMAIL PROTECTED] wrote: On Tue, Sep 30, 2008 at 3:49 PM, Freeland Abbott [EMAIL PROTECTED] wrote: Mostly LGTM. The only (marginally) substantive complaint is that it may make sense to rework the property parsing around lines 285-293 so that it's done once, statically, rather than re-parsing the Ok. I can't really do it in a static intiializer since I may need a TreeLogger, but I can only do it the first time. Or you'd have to introduce a new static method, and call that from somewhere early (i.e. after we have a TreeLogger, but before we're doing anything that might loop over calls). - Can we note, probably in the comments around lines 58-66, that the shard size is measured in number of reachable types, not in e.g. characters? (Yeah, I know characters would be hard to get at *a priori *, but since our issue seems to be string length of the JSNI body, that's what I expected size to be measured in.) The comment for DEFAULT_CREATEMETHODMAP_SHARD_SIZE currently says: Default number of types to split createMethodMap entries into. Zero means no sharding occurs. How are you suggesting that be rewritten? Nevermind. :-/ That answers it; I just misread. - Also mentioned above, line 285-286, why avoid the System.getProperty(String prop, String default) formulation? (You'd have to make the default a string, yes, but it saves you the if-null test.) So you would prefer doing: String shardSizeProperty = System.getProperty(GWT_CREATEMETHODMAP_SHARD_SIZE, String.valueOf(DEFAULT_CREATEMETHODMAP_SHARD_SIZE)); Yes, I like this better than the set-to-default-int, fetch-property, test-for-null sequence. Though my formulation had converted default to String, rather than using String.valueOf(), which gets us to your next comment... This would always require converting the default to a string (or storing it as a string and losing type safety) and always parsing the result rather than a test telling you the parsing is needed. It doesn't seem to be an improvement to me. There's a reason it's in the nits category, and if you think it's a lose, I don't mind if you skip the suggestion. But, to make the case: I see one all-String code path as cleaner than two paths with different data data types on each fork. I'll grant that the integer-fork else of your if is degenerate, but it takes a couple bytes of code size to do the test. Equally, surely it's not a real problem to ensure that the compiled-in default is in fact an integer (that is, the used-once compiled-in string's type safety is a small issue, even among small issues), and we need to handle non-integer values gracefully anyway (because the user can override). But, again, it's a nit: my way saves at most ~10b of code size and could gain or lose a few miliseconds in execution time depending on whether branch prediction in the JVM is so far below bytecode interpretation that misses appear free; your way is type-safe for a constant that's both clearly documented as an integral value and likely to change exactly once in its coded lifetime. What color dresses are the angels on that pin wearing? --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: code review requested, shard TypeSerializerCreator createMethodMap
On Tue, Sep 30, 2008 at 4:51 PM, Freeland Abbott [EMAIL PROTECTED]
wrote:
But, again, it's a nit: my way saves at most ~10b of code size and could
gain or lose a few miliseconds in execution time depending on whether branch
prediction in the JVM is so far below bytecode interpretation that misses
appear free; your way is type-safe for a constant that's both clearly
documented as an integral value and likely to change exactly once in its
coded lifetime. What color dresses are the angels on that pin wearing?
Ok, try this patch.
--
John A. Tamplin
Software Engineer (GWT), Google
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
Index: user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
===
--- user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java (revision 3693)
+++ user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java (working copy)
@@ -19,6 +19,7 @@
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.ext.GeneratorContext;
import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.core.ext.typeinfo.JClassType;
import com.google.gwt.core.ext.typeinfo.JMethod;
import com.google.gwt.core.ext.typeinfo.JParameterizedType;
@@ -32,6 +33,8 @@
import com.google.gwt.user.rebind.SourceWriter;
import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
/**
* This class generates a class with name 'typeSerializerClassName' that is able
@@ -52,6 +55,39 @@
+ SerializationStreamWriter streamWriter, Object instance, String typeSignature)
+ throws SerializationException;
+ /**
+ * Default number of types to split createMethodMap entries into. Zero means
+ * no sharding occurs. Stored as a string since it is used as a default
+ * property value.
+ */
+ private static final String DEFAULT_CREATEMETHODMAP_SHARD_SIZE = 0;
+
+ /**
+ * Java system property name to override the above.
+ */
+ private static final String GWT_CREATEMETHODMAP_SHARD_SIZE = gwt.typecreator.shard.size;
+
+ private static int shardSize = -1;
+
+ private static void computeShardSize(TreeLogger logger)
+ throws UnableToCompleteException {
+String shardSizeProperty = System.getProperty(
+GWT_CREATEMETHODMAP_SHARD_SIZE, DEFAULT_CREATEMETHODMAP_SHARD_SIZE);
+try {
+ shardSize = Integer.valueOf(shardSizeProperty);
+ if (shardSize 0) {
+logger.log(TreeLogger.ERROR, GWT_CREATEMETHODMAP_SHARD_SIZE
++ must be non-negative: + shardSizeProperty);
+throw new UnableToCompleteException();
+ }
+} catch (NumberFormatException e) {
+ logger.log(TreeLogger.ERROR, Property
+ + GWT_CREATEMETHODMAP_SHARD_SIZE + not a number:
+ + shardSizeProperty, e);
+ throw new UnableToCompleteException();
+}
+ }
+
private final GeneratorContext context;
private final JType[] serializableTypes;
@@ -66,7 +102,7 @@
public TypeSerializerCreator(TreeLogger logger,
SerializableTypeOracle serializationOracle, GeneratorContext context,
- String typeSerializerClassName) {
+ String typeSerializerClassName) throws UnableToCompleteException {
this.context = context;
this.typeSerializerClassName = typeSerializerClassName;
this.serializationOracle = serializationOracle;
@@ -75,9 +111,14 @@
serializableTypes = serializationOracle.getSerializableTypes();
srcWriter = getSourceWriter(logger, context);
+if (shardSize 0) {
+ computeShardSize(logger);
+}
+logger.log(TreeLogger.TRACE, Using a shard size of + shardSize
++ for TypeSerializerCreator createMethodMap);
}
- public String realize(TreeLogger logger) {
+ public String realize(TreeLogger logger) throws UnableToCompleteException {
logger = logger.branch(TreeLogger.DEBUG,
Generating TypeSerializer for service interface '
+ getTypeSerializerClassName() + ', null);
@@ -92,7 +133,7 @@
writeCreateMethods();
-writeCreateMethodMapMethod();
+writeCreateMethodMapMethod(logger);
writeCreateSignatureMapMethod();
@@ -173,13 +214,13 @@
packageName = className.substring(0, index);
className = className.substring(index + 1, className.length());
}
-return new String[]{packageName, className};
+return new String[] {packageName, className};
}
-
+
private JType[] getSerializableTypes() {
return serializableTypes;
}
-
+
private SourceWriter getSourceWriter(TreeLogger logger, GeneratorContext ctx) {
String name[] = getPackageAndClassName(getTypeSerializerClassName());
String packageName = name[0];
@@ -207,6 +248,16 @@
}
/**
+ * @param type
+ * @return
+ */
+
[gwt-contrib] Re: RR: Hosted mode accepts 'undefined' or 'null' for boolean primitive type
Thanks Scott - I'm glad you were following the thread! On Sat, Sep 27, 2008 at 2:20 AM, Scott Blum [EMAIL PROTECTED] wrote: On Fri, Sep 26, 2008 at 11:37 AM, Eric Ayers [EMAIL PROTECTED] wrote: (arg2 == null ? false : arg2) Use the !! coerce-to-boolean idiom instead. -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: DatePicker review
Something like this?
protected void refreshAll() {
getDatePicker().refresh();
}
Yep, and baring unexpected events, all the changes should be committed into
incubator trunk tomorrow. Thanks for your help in pounding on it!
On Mon, Sep 29, 2008 at 5:18 PM, Isaac Truett [EMAIL PROTECTED] wrote:
Yeah, 99% of that code (I'm estimating...) is CP from the Default
classes. The biggest difference from gen1 seems to be that more code
is package-protected than before. I had to copy DefaultCalendarView
even though I didn't have any change to make in it. Other seemingly
unnecessary copies are the Css, CssHandling, and DateStyler inner
classes of DatePicker.
I've started to rewrite addMonths() in my MonthSelector and ran into
another package-protected method, DatePicker.refresh(), and that's as
far as I had gotten before having to return to real work.
On Mon, Sep 29, 2008 at 5:03 PM, Emily Crutcher [EMAIL PROTECTED] wrote:
I doubt it, as I'm still finishing the code to create it :-). Can you
tell
me the pain points? As making it easy to extend is something we are
committed to.Are you counting the code to cut/copy the default
calendar
and month selectors?
On Mon, Sep 29, 2008 at 4:44 PM, Isaac Truett [EMAIL PROTECTED]
wrote:
Does anyone have a working extension of the gen2 DatePicker? I
haven't
had a lot of time to look at it so I may be doing something horribly
wrong but it seems that extending it is much more complicated than it
was with the old DatePicker. I have almost 500 lines of code so far
and I still haven't finished -- all just to add drop down lists for
month and year. With the old DatePicker, I did it in under 150 lines.
On Mon, Sep 29, 2008 at 1:27 PM, Emily Crutcher [EMAIL PROTECTED]
wrote:
Could you review the gen 2 date picker code? The code is in
com.google.gwt.gen2.datepicker. This code is going to be moved over
to
gwt
after the handler code is moved, so nitpicks would be very welcome.
thanks!
Emily
--
There are only 10 types of people in the world: Those who
understand
binary, and those who don't
--
There are only 10 types of people in the world: Those who understand
binary, and those who don't
--
There are only 10 types of people in the world: Those who understand
binary, and those who don't
--
There are only 10 types of people in the world: Those who understand
binary, and those who don't
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: code review requested, shard TypeSerializerCreator createMethodMap
LGTM On Tue, Sep 30, 2008 at 5:26 PM, John Tamplin [EMAIL PROTECTED] wrote: On Tue, Sep 30, 2008 at 4:51 PM, Freeland Abbott [EMAIL PROTECTED] wrote: But, again, it's a nit: my way saves at most ~10b of code size and could gain or lose a few miliseconds in execution time depending on whether branch prediction in the JVM is so far below bytecode interpretation that misses appear free; your way is type-safe for a constant that's both clearly documented as an integral value and likely to change exactly once in its coded lifetime. What color dresses are the angels on that pin wearing? Ok, try this patch. -- John A. Tamplin Software Engineer (GWT), Google --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
