[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
LGTM++
On Thu, Apr 28, 2011 at 10:27 AM, wrote:
>
>
> http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
> File
> user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
> (right):
>
>
> http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52
>
> user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52:
> assertEquals("Hello Bob", domUi.div.getInnerHTML());
> On 2011/04/27 23:21:30, rjrjr wrote:
>
>> You might want call .toLowerCase() on both strings to normalize. IE
>>
> does funny
>
>> things to tag names, you'll get dinged in the cross browser tests.
>>
>
> Done.
>
>
> http://gwt-code-reviews.appspot.com/1425811/
>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
File
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52:
assertEquals("Hello Bob", domUi.div.getInnerHTML());
On 2011/04/27 23:21:30, rjrjr wrote:
You might want call .toLowerCase() on both strings to normalize. IE
does funny
things to tag names, you'll get dinged in the cross browser tests.
Done.
http://gwt-code-reviews.appspot.com/1425811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
LGTM TextBox isn't in the fake widget set defined by com.google.gwt.uibinder.test.UiJavaResources, and Label is. Your new code reports such errors now, which is great. http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
File
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52:
assertEquals("Hello Bob", domUi.div.getInnerHTML());
You might want call .toLowerCase() on both strings to normalize. IE does
funny things to tag names, you'll get dinged in the cross browser tests.
http://gwt-code-reviews.appspot.com/1425811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
I'll patch this in and try to figure out the TextBox thing. http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
File
user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java#newcode91
user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java:91:
return writer.tokenForSafeHtmlExpression(
On 2011/04/27 19:29:58, rjrjr wrote:
I think instead you want to use the tokenForExpression method that
Rafa just
added.
Done.
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java
File
user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java#newcode98
user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java:98:
b.append(" ");
On 2011/04/27 19:29:58, rjrjr wrote:
This is a bit disturbing. Why did it fix it? Or is the real question
why did it
pass in the first place?
Sorry - I thought you saw my comment on the other review thread - I have
no idea why this didn't work for TextBox and was hoping that perhaps you
would have some inkling?
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml
File
user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml#newcode18
user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml:18:
On 2011/04/27 19:29:58, rjrjr wrote:
I don't think you need the entry-point
Done.
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
File
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52:
assertEquals("Hello Bob", domUi.div.getInnerHTML());
On 2011/04/27 19:29:58, rjrjr wrote:
check for the , make sure it didn't get escaped.
Done.
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java
File user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java#newcode28
user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java:28:
return "Hello " + name;
On 2011/04/27 19:29:58, rjrjr wrote:
Should put some markup in here, so we can test for improper escaping.
"Hello
" + name + "."
Done.
http://gwt-code-reviews.appspot.com/1425811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
File
user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java#newcode91
user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java:91:
return writer.tokenForSafeHtmlExpression(
I think instead you want to use the tokenForExpression method that Rafa
just added.
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java
File
user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java#newcode98
user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java:98:
b.append(" ");
This is a bit disturbing. Why did it fix it? Or is the real question why
did it pass in the first place?
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml
File
user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml#newcode18
user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml:18:
I don't think you need the entry-point
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
File
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52
user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52:
assertEquals("Hello Bob", domUi.div.getInnerHTML());
check for the , make sure it didn't get escaped.
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java
File user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java
(right):
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java#newcode28
user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java:28:
return "Hello " + name;
Should put some markup in here, so we can test for improper escaping.
"Hello " + name + "."
http://gwt-code-reviews.appspot.com/1425811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
