[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
On Mon, Nov 9, 2009 at 8:51 PM, Joel Webber wrote: > And just to be clear, we recognize that it's a real problem, but I think > it's going to take some time to work out how to shove the iframe shim in, in > such a way that it doesn't break existing code (the IE6/7 implementation > takes advantage of CSS expressions to keep the iframe positioned correctly). Well, almost: http://code.google.com/p/google-web-toolkit/issues/detail?id=2907 ;-) > Of particular concern to me is the case where you're just using it as a > simple tooltip-esque popup and *don't* want anything (i.e. glass) covering > the whole page. Sure, but when you use the optional glass, the iframe should cover the same surface (whole window), not only the popup. > The closure dialog implementation assumes that you *do* want > such a thing, so the problem's a bit easier there. Agreed; and I'd note that Closure doesn't use a shim for popups (in neither PopupBase or Popup), only for DialogBox. > I'm very, very open to ideas on this. I think enabling the shim only in IE6 is ok for now, given GWT's "goal" with this shim. I'll try porting Closure's DialogBox to GWT because it's exactly what I need (except that I also need the dialog to contain widgets, but that's just a detail ;-) ). As for what GWT should do in the end, well, I have no idea for now, sorry... -- Thomas Broyer /tɔ.ma.bʁwa.je/ --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
And just to be clear, we recognize that it's a real problem, but I think
it's going to take some time to work out how to shove the iframe shim in, in
such a way that it doesn't break existing code (the IE6/7 implementation
takes advantage of CSS expressions to keep the iframe positioned correctly).
Of particular concern to me is the case where you're just using it as a
simple tooltip-esque popup and *don't* want anything (i.e. glass) covering
the whole page. The closure dialog implementation assumes that you *do* want
such a thing, so the problem's a bit easier there.
I'm very, very open to ideas on this.
On Mon, Nov 9, 2009 at 2:47 PM, wrote:
> Reviewers: jgw, t.broyer,
>
> Message:
> Okay, we'll back off from this one.
>
>
>
> Please review this at http://gwt-code-reviews.appspot.com/97807
>
> Affected files:
>
> M user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
>
>
> Index: user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> diff --git a/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> b/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> index
> 74d28b3811c4d4d2b58de6322024900e4c5a82f4..9d7a696598c4eb74322bca932c7c178282833a64
> 100644
> --- a/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> +++ b/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> @@ -22,9 +22,53 @@ import com.google.gwt.user.client.Element;
> * {...@link com.google.gwt.user.client.ui.impl.PopupImpl}.
> */
> public class PopupImplIE6 extends PopupImpl {
> + private static boolean isIE6 = isIE6();
> +
> + // Stolen and modified from UserAgent.gwt.xml.
> + // TODO(jgw): Get rid of this method, and switch to using soft
> permutations
> + // once they land in trunk.
> + private static native boolean isIE6() /*-{
> + function makeVersion(result) {
> + return (parseInt(result[1]) * 1000) + parseInt(result[2]);
> + }
> +
> + var ua = navigator.userAgent.toLowerCase();
> + if (ua.indexOf("msie") != -1) {
> + var result = /msie ([0-9]+)\.([0-9]+)/.exec(ua);
> + if (result && result.length == 3) {
> + var v = makeVersion(result);
> + if (v < 7000) {
> + return true;
> + }
> + }
> + }
> +
> + return false;
> + }-*/;
> +
> +
> + @Override
> + public void onHide(Element popup) {
> +if (isIE6) {
> + do_onHide(popup);
> +}
> + }
> +
> + @Override
> + public void onShow(Element popup) {
> +if (isIE6) {
> + do_onShow(popup);
> +}
> + }
>
> @Override
> - public native void onHide(Element popup) /*-{
> + public void setVisible(Element popup, boolean visible) {
> +if (isIE6){
> + do_setVisible(popup, visible);
> +}
> + }
> +
>
> + private native void do_onHide(Element popup) /*-{
> // It is at least rarely possible to get an onHide() without a matching
> // onShow(), usually because of timing issues created by animations. So
> // we're careful not to assume the existence of '__frame' here.
> @@ -35,9 +79,8 @@ public class PopupImplIE6 extends PopupImpl {
> popup.__frame = null;
> }
> }-*/;
> -
> - @Override
> - public native void onShow(Element popup) /*-{
> +
> + private native void do_onShow(Element popup) /*-{
> // TODO: make this more Java and less JSNI?
> var frame = $doc.createElement('iframe');
>
> @@ -82,8 +125,7 @@ public class PopupImplIE6 extends PopupImpl {
> popup.parentElement.insertBefore(frame, popup);
> }-*/;
>
> - @Override
> - public native void setVisible(Element popup, boolean visible) /*-{
> + private native void do_setVisible(Element popup, boolean visible) /*-{
> if (popup.__frame) {
> popup.__frame.style.visibility = visible ? 'visible' : 'hidden';
> }
>
>
>
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
Reviewers: jgw, t.broyer,
Message:
Okay, we'll back off from this one.
Please review this at http://gwt-code-reviews.appspot.com/97807
Affected files:
M user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
Index: user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
diff --git a/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
b/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
index
74d28b3811c4d4d2b58de6322024900e4c5a82f4..9d7a696598c4eb74322bca932c7c178282833a64
100644
--- a/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
+++ b/user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
@@ -22,9 +22,53 @@ import com.google.gwt.user.client.Element;
* {...@link com.google.gwt.user.client.ui.impl.PopupImpl}.
*/
public class PopupImplIE6 extends PopupImpl {
+ private static boolean isIE6 = isIE6();
+
+ // Stolen and modified from UserAgent.gwt.xml.
+ // TODO(jgw): Get rid of this method, and switch to using soft
permutations
+ // once they land in trunk.
+ private static native boolean isIE6() /*-{
+ function makeVersion(result) {
+ return (parseInt(result[1]) * 1000) + parseInt(result[2]);
+ }
+
+ var ua = navigator.userAgent.toLowerCase();
+ if (ua.indexOf("msie") != -1) {
+ var result = /msie ([0-9]+)\.([0-9]+)/.exec(ua);
+ if (result && result.length == 3) {
+ var v = makeVersion(result);
+ if (v < 7000) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }-*/;
+
+
+ @Override
+ public void onHide(Element popup) {
+if (isIE6) {
+ do_onHide(popup);
+}
+ }
+
+ @Override
+ public void onShow(Element popup) {
+if (isIE6) {
+ do_onShow(popup);
+}
+ }
@Override
- public native void onHide(Element popup) /*-{
+ public void setVisible(Element popup, boolean visible) {
+if (isIE6){
+ do_setVisible(popup, visible);
+}
+ }
+
+ private native void do_onHide(Element popup) /*-{
// It is at least rarely possible to get an onHide() without a matching
// onShow(), usually because of timing issues created by animations. So
// we're careful not to assume the existence of '__frame' here.
@@ -35,9 +79,8 @@ public class PopupImplIE6 extends PopupImpl {
popup.__frame = null;
}
}-*/;
-
- @Override
- public native void onShow(Element popup) /*-{
+
+ private native void do_onShow(Element popup) /*-{
// TODO: make this more Java and less JSNI?
var frame = $doc.createElement('iframe');
@@ -82,8 +125,7 @@ public class PopupImplIE6 extends PopupImpl {
popup.parentElement.insertBefore(frame, popup);
}-*/;
- @Override
- public native void setVisible(Element popup, boolean visible) /*-{
+ private native void do_setVisible(Element popup, boolean visible) /*-{
if (popup.__frame) {
popup.__frame.style.visibility = visible ? 'visible' : 'hidden';
}
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
Agreed -- we're going to do precisely that. I think we'll probably end up
sticking isIE6() in the image bundle code for the moment, as Bob has a
better general solution in the pipeline (which he calls soft permutations).
But yes, the image bundle insanity on IE6 is the more important of the two
that we wanted to fix.
On Mon, Nov 9, 2009 at 11:01 AM, stuckagain wrote:
>
> I'm not a code reviewer... but just a useful comment:
>
> You might want to move the isIE6 method somewhere else so that you
> could avoid the memory leak on ie7/ie8 when using ImageBundles as
> well.
> For ie7 and ie8 this workaround to get transparent PNGs working is no
> longer needed and it's leaking a lot of memory. Unless you want to
> wait for soft permutations (first time that I hear that mentioned).
>
> See the following:
> http://code.google.com/p/google-web-toolkit/issues/detail?id=3573
> http://code.google.com/p/google-web-toolkit/issues/detail?id=3588
>
> David
>
> On Nov 9, 3:22 pm, j...@google.com wrote:
> > LGTM. Thanks.
> >
> > http://gwt-code-reviews.appspot.com/97807/diff/3/1002
> > File user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> > (right):
> >
> > http://gwt-code-reviews.appspot.com/97807/diff/3/1002#newcode71
> > Line 71: private native void do_onHide(Element popup) /*-{
> > I know these are private, but do_onHide()? Can we just make them
> > doOnHide()?
> >
> > http://gwt-code-reviews.appspot.com/97807
> >
>
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
I'm not a code reviewer... but just a useful comment:
You might want to move the isIE6 method somewhere else so that you
could avoid the memory leak on ie7/ie8 when using ImageBundles as
well.
For ie7 and ie8 this workaround to get transparent PNGs working is no
longer needed and it's leaking a lot of memory. Unless you want to
wait for soft permutations (first time that I hear that mentioned).
See the following:
http://code.google.com/p/google-web-toolkit/issues/detail?id=3573
http://code.google.com/p/google-web-toolkit/issues/detail?id=3588
David
On Nov 9, 3:22 pm, j...@google.com wrote:
> LGTM. Thanks.
>
> http://gwt-code-reviews.appspot.com/97807/diff/3/1002
> File user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
> (right):
>
> http://gwt-code-reviews.appspot.com/97807/diff/3/1002#newcode71
> Line 71: private native void do_onHide(Element popup) /*-{
> I know these are private, but do_onHide()? Can we just make them
> doOnHide()?
>
> http://gwt-code-reviews.appspot.com/97807
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
On 2009/11/09 14:49:12, jgw wrote: > On 2009/11/09 14:34:54, t.broyer wrote: > > Sorry to jump in without having been invited but, given > > http://code.google.com/p/google-web-toolkit/issues/detail?id=805 (among > others), > > shouldn't the iframe shim be extended to all browsers instead? > > > > (Closure-library seems to be doing just this; or actually, it moves the > > responsibility to the developer on a case-by-case basis: > > http://closure-library.googlecode.com/svn/trunk/closure/goog/docs/class_goog_ui_Dialog.html > > ) > [comments always welcome!] > Issue 805 seems to be primarily about Flash. I haven't had time to look into > this, but I thought you could add a parameter to the Flash object to fix this > (one commenter suggests wmode='transparent', which seems to ring a bell. If this > proves insufficient, I would be happy to revisit the problem. There's also the issue with PDFs and applets: http://code.google.com/p/google-web-toolkit/issues/detail?id=3268 http://code.google.com/p/google-web-toolkit/issues/detail?id=673 http://code.google.com/p/google-web-toolkit/issues/detail?id=1662 (the last two have been merged with issue 805, and Ray commented on 673 "Marking as a dup of 805, which proposes a solution", the solution being to use the iframe shim in all browsers) Oh, and as a side-note, have a look at my recent comment on issue 2907 if you have time, about the iframe shim and the new popup glass pane: http://code.google.com/p/google-web-toolkit/issues/detail?id=2907#c4 (the shim should cover the whole window, just like the glass pane, or it won't fully do its work) http://gwt-code-reviews.appspot.com/97807 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
On 2009/11/09 14:34:54, t.broyer wrote: > Sorry to jump in without having been invited but, given > http://code.google.com/p/google-web-toolkit/issues/detail?id=805 (among others), > shouldn't the iframe shim be extended to all browsers instead? > (Closure-library seems to be doing just this; or actually, it moves the > responsibility to the developer on a case-by-case basis: http://closure-library.googlecode.com/svn/trunk/closure/goog/docs/class_goog_ui_Dialog.html > ) [comments always welcome!] Issue 805 seems to be primarily about Flash. I haven't had time to look into this, but I thought you could add a parameter to the Flash object to fix this (one commenter suggests wmode='transparent', which seems to ring a bell. If this proves insufficient, I would be happy to revisit the problem. http://gwt-code-reviews.appspot.com/97807 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Squelch ie6 popup iframe hack on ie7
Sorry to jump in without having been invited but, given http://code.google.com/p/google-web-toolkit/issues/detail?id=805 (among others), shouldn't the iframe shim be extended to all browsers instead? (Closure-library seems to be doing just this; or actually, it moves the responsibility to the developer on a case-by-case basis: http://closure-library.googlecode.com/svn/trunk/closure/goog/docs/class_goog_ui_Dialog.html ) http://gwt-code-reviews.appspot.com/97807 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
