On Thursday, November 3, 2016 at 5:33:31 PM UTC+1, Colin Alworth wrote: > > With 3.0 on the horizon, we've promised consistency of a sort in 2.x, and > without 3.0 actually in sight, 2.x is going to need to see active > development. Encouraging a third generation of url tools is not a bad > thing, but only switching over half-way leaves something to be desired. > > I'll play devil's advocate a bit - I'm not addressing the patch > specifically (since I haven't fully read all of it or the discussion around > it, and if it was a bad patch, I'm sure it would have been shot down on its > own merits) but the thinking to use around what goes into the 2.x branch: > * (point 1) We're not ready for maintenance mode, at least until we have > timelines and completed APIs for GWT 3. If we are, we should be forbidding > all merge requests to gwt-user that don't fix critical bugs. > > * (point 2 and 3) Can't argue with new tools arriving that solve the > problem better, especially if they are going into 3 as a cross-platform way > of solving the problem (gwt/java/j2cl). Obvious caveat here (even for the > devil's advocate) that with the release of 3, breaking changes off of 2 are > acceptable and expected. > > * (point 4) Without transitioning the existing GWT Safe* tools, > SafeHtmlTemplates is stuck in the past, as is UiBinder, I18n Messages, and > any HasSafeHtml widget (both in GWT and in the general ecosystem). This is > a lot to leave behind in the name of "but the tool didn't belong in GWT in > the first place". We've had our chance to properly deprecate it for the 2.8 > release, and if our past timelines are any measure, it is going to be at > least a year before we finally ship a release with SafeHtml, and all that > use it, deprecated. And once we've reached that point, unless gwt-user > depends on (or rebases, etc) safe-html-types or any other successor, all of > the above downstream classes which use SafeHtml are left effectively broken > (not unlike the unported Generators and Linkers we have today). > > Obviously ClientBundle/GssResource isn't actually deprecated, but we have > officially said that new tools should not use the features that it takes > advantage of. SafeHtml takes this a step further - GssResource got several > upgrades within the 2.8 release (unlike other generators), despite it being > deprecated, but with SafeHtml going out, it takes out features within many > GWT Widgets themselves. There will be no officially supported way to > correctly assign safe html content into an HTML widget. > > Now yes, a bit of hyperbole there - you can of course (as Thomas noted in > his email) simply asString() the not-gwt SafeHtml and use it, or provide > your own wrapper, but depending on your project size (and GWT users out > there have some big ones) that could be hundreds or thousands of sites to > fix in code. Yet another change would be needed for any widgets that the > project has which implement HasSafeHtml (so it can still be used in > UiBinder), UiBinder @UiFields or getters (which return SafeUri for use in > its embedded SafeHtml handling), Messages (to wrap any incoming url). This > ignores any use of Messages/Constants in a SafeHtml-using uibinder itself - > I'm not seeing a clear path there without the use of default methods in > I18n interfaces (which admittedly, would probably work, but isn't going to > be pretty). >
You forgot one major point here: this patch is proposing a new API (new overloads to existing methods), without actually changing the current API (at least in terms of its contract). So it's not about changing "every call sites for SafeHtml/UriUtils API", it's about being able to take advantage of a new behavior by selectively calling a new API. If you want that behavior everywhere, then yes you'll have to update all call sites; but the patch doesn't change that in any way (except what you change your code *to*) All those existing usage of SafeHtml you cited (SafeHtmlTemplates, UiBinder, I18N and HasSafeHtml) would be unchanged and keeping their current behavior (and only 2 of them deal with SafeUri: SafeHtmlTemplates and UiBinder). And the only two cases where UriUtils is called by those (vs. receiving a SafeUri value from the "user") are discouraged and generate a warning (and would continue to only use the default whitelist and reject "tel:" URIs; if you'd like to allow "tel:" URIs there, you'd have to change to SafeUri anyway, and then the responsibility of converting a String to a SafeUri falls on you, so no change to UriUtils or other APIs/generators is actually *required*): https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f587fcfee16/user/src/com/google/gwt/uibinder/attributeparsers/SafeUriAttributeParser.java#L41 https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f587fcfee16/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#L183 (warning at: https://github.com/gwtproject/gwt/blob/3a41a8067767965ff72f4e49eb015f587fcfee16/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#L364) After the security review, we're not in a situation of "I think GWT's behavior should be changed (and this will affect everyone in many places)", we're rather in a situation of "I'd like to add this new feature to GWT; and this is something I could do in my own code" (delegating to the existing behavior after I checked whether the new behavior I want would apply). To put it in precise terms for this patch, it's a matter of changing UriUtils.fromString(…) to either TelAwareUriUtils.fromString(…) (application-specific factory that eventually delegate to UriUtils.fromString) or UriUtils.fromString(…, Collections.singleton("tel")) (new proposed API). That TelAwareUriUtils would look like: public class TelAwareUriUtils { public static SafeUri fromString(String uri) { if (isTelUri(uri)) { return UriUtils.fromTrustedString(uri); } return UriUtils.fromString(uri); } // add similar isSafeUri and sanitizeUri methods private boolean isTelUri(String uri) { return uri.regionMatches(true, 0, "tel:", 0, 4); // this is a case-insensitive startsWith } } Speaking for myself again, I am not prepared to outright "-2" any > contribution to gwt-user that isn't a fix for a critical bug. I don't think > we're ready today to deprecate everything inside of gwt-user that isn't JRE > emulation or jsinterop-based. Ideally of course aspects of GWT will be spun > out as community projects and maintained separately, but until we pull it > off (or even start), any new GWT user will assume that anything in gwt-user > is the best basis for a GWT project. Actively abandoning most of gwt-user > is not going to help the already difficult situation we're in there. > +1, but we're not in that exact situation here. -- 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/cede6c7f-7428-42f0-bc42-8b1cb03cbecf%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
