The comment is just to indicate that IE9 is bound to legacy FocusImplIE6 only because we didn't test is with a more modern implementation. So later when somebody looks at the code for cleanup will not think that we carefully tested and decided to use the Iegacy implementation.
On Thu, Oct 2, 2014 at 10:07 AM, Colin Alworth <[email protected]> wrote: > Sorry Goktug, I don't follow - what should the comment indicate? In the > old code, IE9 was expressly bound to the IE6 imp, in the new (to be > reverted) code, it was expressly *not* bound to the IE6 impl, but it was > the closest fallback anyway. > > Are you suggesting that the comment should point out that IE9 probably > shouldn't be bound in that way (though it works, and may not work if > differently bound)? > > Thanks for any clarification, > Colin > > On Thu, Oct 2, 2014 at 11:05 AM, 'Goktug Gokdogan' via GWT Contributors < > [email protected]> wrote: > >> Reverting the binding sounds good to me. Please add a comment to the >> binding; it might probably be bound to something else but not tested. >> >> On Wed, Oct 1, 2014 at 9:39 PM, Colin Alworth <[email protected]> wrote: >> >>> It looks like changes to a few rebind rules are generating some new >>> logspam when any GWT app compiles. Specifically, I'm seeing FocusImpl and >>> LayoutImpl, though its possible there are others I haven't seen yet. From >>> the dynatable example we can see the FocusImpl spam: >>> gwtc: >>> [java] Compiling module com.google.gwt.sample.dynatable.DynaTable >>> [java] Computing all possible rebind results for >>> 'com.google.gwt.user.client.ui.impl.FocusImpl' >>> [java] Rebinding com.google.gwt.user.client.ui.impl.FocusImpl >>> * [java] Could not find an exact match rule. Using >>> 'closest' rule <replace-with >>> class='com.google.gwt.user.client.ui.impl.FocusImplIE6'/> based on fall >>> back values. You may need to implement a specific binding in case the fall >>> back behavior does not replace the missing binding* >>> [java] Compiling 5 permutations >>> [java] Compiling permutation 0... >>> [java] Process output >>> [java] Compiling >>> [java] Compiling permutation 1... >>> [java] Compiling permutation 2... >>> [java] Compiling permutation 3... >>> >>> >>> First, the rules themselves: >>> <!-- Firefox uses a hidden input to set accesskeys --> >>> <replace-with >>> class="com.google.gwt.user.client.ui.impl.FocusImplStandard"> >>> <when-type-is class="com.google.gwt.user.client.ui.impl.FocusImpl"/> >>> <when-property-is name="user.agent" value="gecko1_8"/> >>> </replace-with> >>> >>> <!-- Safari uses a hidden input to set accesskeys and --> >>> <!-- fires focus/blur after a timeout --> >>> <replace-with >>> class="com.google.gwt.user.client.ui.impl.FocusImplSafari"> >>> <when-type-is class="com.google.gwt.user.client.ui.impl.FocusImpl"/> >>> <when-property-is name="user.agent" value="safari"/> >>> </replace-with> >>> >>> <!-- IE's implementation traps exceptions on invalid setFocus() --> >>> <replace-with class="com.google.gwt.user.client.ui.impl.FocusImplIE6"> >>> <when-type-is class="com.google.gwt.user.client.ui.impl.FocusImpl"/> >>> <all> >>> <any> >>> <when-property-is name="user.agent" value="ie6"/> >>> <when-property-is name="user.agent" value="ie8"/> >>> </any> >>> <none> >>> <when-property-is name="user.agent" value="ie9"/> >>> </none> >>> </all> >>> </replace-with> >>> >>> No express binding of FocusImpl to itself, or to Standard (interestingly >>> Standard really seems to mean Gecko, at least according to that comment, >>> calling out a FF specific issue). I can't comment much on that just yet, >>> though I might make a suggestion later. >>> >>> This code is as a result of >>> https://gwt-review.googlesource.com/#/c/5055/ (or >>> https://gwt.googlesource.com/gwt/+/040b3e4392186e48689a6fc1f19cdf294f2b5651 >>> if you like), ostensibly to make IE9+ use FocusImpl instead of FocusImplIE6 >>> (or Standard, for example). >>> >>> [java] Computing all possible rebind results for >>> 'com.google.gwt.user.client.ui.impl.FocusImpl' >>> >>> ... >>> >>> [java] Found better fallback match for <replace-with >>> class='com.google.gwt.user.client.ui.impl.FormPanelImpl'/> >>> [java] Checking rule <replace-with >>> class='com.google.gwt.user.client.ui.impl.FocusImplIE6'/> >>> [java] Checking if all subconditions are true (<all>) >>> [java] <when-type-is >>> class='com.google.gwt.user.client.ui.impl.FocusImpl'/> >>> [java] Yes, the requested type was an exact match >>> [java] Checking if all subconditions are true (<all>) >>> [java] Checking if any subcondition is true (<any>) >>> [java] <when-property-is name='user.agent' >>> value='ie8'/> >>> [java] Property value is '*ie9*' >>> [java] Property value 'ie8' is the fallback of >>> '[[ie9]]' >>> [java] No, the value did not match >>> [java] No: All subconditions were false >>> [java] No: One or more subconditions was false >>> [java] No: One or more subconditions was false >>> [java] Rule did not match >>> [java] Found better fallback match for <replace-with >>> class='com.google.gwt.user.client.ui.impl.FocusImplIE6'/> >>> [java] Checking rule <replace-with >>> class='com.google.gwt.user.client.ui.impl.FocusImplSafari'/> >>> [java] Checking if all subconditions are true (<all>) >>> [java] <when-type-is >>> class='com.google.gwt.user.client.ui.impl.FocusImpl'/> >>> [java] Yes, the requested type was an exact match >>> [java] <when-property-is name='user.agent' >>> value='safari'/> >>> [java] Property value is 'ie9' >>> [java] No, the value did not match >>> [java] No: One or more subconditions was false >>> [java] Rule did not match >>> [java] Checking rule <replace-with >>> class='com.google.gwt.user.client.ui.impl.FocusImplStandard'/> >>> [java] Checking if all subconditions are true (<all>) >>> [java] <when-type-is >>> class='com.google.gwt.user.client.ui.impl.FocusImpl'/> >>> [java] Yes, the requested type was an exact match >>> [java] <when-property-is name='user.agent' >>> value='gecko1_8'/> >>> [java] Property value is 'ie9' >>> [java] No, the value did not match >>> [java] No: One or more subconditions was false >>> [java] Rule did not match >>> >>> ... >>> >>> [java] Could not find an exact match rule. Using 'closest' rule >>> <replace-with class='com.google.gwt.user.client.ui.impl.FocusImplIE6'/> >>> based on fall back values. You may need to implement a specific binding in >>> case the fall back behavior does not replace the missing binding >>> [java] No exact match was found, using closest match rule >>> <replace-with class='com.google.gwt.user.client.ui.impl.FocusImplIE6'/> >>> [java] Rebind result was >>> com.google.gwt.user.client.ui.impl.FocusImplIE6 >>> >>> From this abbreviated log, apparently IE9 is still using the IE6 impl, >>> rather than ignoring any rebind rule and using the actual argument to >>> GWT.create. >>> >>> My *guess* is that this has been the case since the patch linked was >>> authored, and checking out 040b3e4392186e48689a6fc1f19cdf294f2b5651 seems >>> to confirm this - building dynatable still has this log message, indicating >>> that IE9 has been using IE6 all along, despite the commit message. The >>> linked issue >>> https://code.google.com/p/google-web-toolkit/issues/detail?id=8383 >>> (which I didn't read until just now, and could have saved me a fair bit of >>> time...) seems to confirm this. >>> >>> My goal in this quick bit of debugging is to get us to a stable GWT >>> release that doesn't add useless logging. To that end, I think >>> 040b3e4392186e48689a6fc1f19cdf294f2b5651 must be reverted, since it has no >>> effect, and adds confusing log messages that might suggest to the user that >>> they have made a mistake. The commit at >>> https://gwt.googlesource.com/gwt/+/a0a0c20713cbc2d0fc8dd262f066da9f2412c93a >>> should get the same treatment (LayoutImpl), and I'll be walking a few more >>> commits to confirm that everything else looks safe. >>> >>> The other option is to declare explicitly to bind FocusImpl to FocusImpl >>> (with or without a 'useragent=ie9' check) so that this is then the closest >>> matching rule. Since we haven't actually been testing IE9 with the 'real' >>> implementation all along, we have no real idea right now what would happen >>> if we did this, so I'm inclined against it. >>> >>> Thoughts or concerns about this issue and any fix? >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "GWT Contributors" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected] >>> . >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/google-web-toolkit-contributors/e1628118-bb35-4d5a-aa31-b9ba1ce070ba%40googlegroups.com >>> <https://groups.google.com/d/msgid/google-web-toolkit-contributors/e1628118-bb35-4d5a-aa31-b9ba1ce070ba%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "GWT Contributors" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/google-web-toolkit-contributors/CAN%3DyUA26VSXTzM9MviU34o80e%3D-NCchT0qdkPu3MSeW04epceA%40mail.gmail.com >> <https://groups.google.com/d/msgid/google-web-toolkit-contributors/CAN%3DyUA26VSXTzM9MviU34o80e%3D-NCchT0qdkPu3MSeW04epceA%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > > > -- > 218.248.6165 > [email protected] > > -- > You received this message because you are subscribed to the Google Groups > "GWT Contributors" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-web-toolkit-contributors/CADcXZMwNNrepDuCuuopGg1rhboctaYvd7mGjFY69o00oPw9GBA%40mail.gmail.com > <https://groups.google.com/d/msgid/google-web-toolkit-contributors/CADcXZMwNNrepDuCuuopGg1rhboctaYvd7mGjFY69o00oPw9GBA%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/google-web-toolkit-contributors/CAN%3DyUA38vOAmCHOGjpPX8rVd7wJXFe%3DwfSPPkF29QMK19uH4KA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
