[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread Thomas Broyer
On Fri, Oct 19, 2012 at 6:12 PM, Matthew Dempsky  wrote:
> On Fri, Oct 19, 2012 at 5:50 AM,  wrote:
>>
>> Except you missed one that spans 2 lines in ImageLoadingCell.
>> (caught using "grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/
>> |grep asString |grep -v setInnerHTML" ;-) )
>
>
> FWIW, I used Error Prone[1] to do this refactoring.  "Not as clumsy or
> random as a regex; an elegant tool, for a more civilized age."  :)
>
> [1] http://code.google.com/p/error-prone/

Oh, so you did develop a specific "bug pattern", right?

(you learn a new thing every day! thanks for the pointer, didn't know
about that project –one day, I should really go through the whole list
of projects tagged with "google" on Google Code Hosting ;-) )

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors


[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread Matthew Dempsky
On Fri, Oct 19, 2012 at 7:00 AM,  wrote:

> Can we deprecate Element.setInnerHTML in favor of setInnerSafeHtml?
> Ideally, we would be able to remove the unsafe versions before long.
>

I'm certainly in favor of encouraging people to use setInnerSafeHtml() or
setInnerText() instead of setInnerHTML().  If deprecation's the best way to
do that, then I'd support that, but I suspect we'll need to do some more
cleanup work still.

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread Matthew Dempsky
On Fri, Oct 19, 2012 at 5:50 AM,  wrote:

> D'oh, where was I when Element#setInnerSafeHtml was added? Didn't know
> it even existed!
>

Yep, I added it last December.  Just finally getting around to cleaning up
the old code though. :)

Except you missed one that spans 2 lines in ImageLoadingCell.
> (caught using "grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/
> |grep asString |grep -v setInnerHTML" ;-) )
>

FWIW, I used Error Prone[1] to do this refactoring.  "Not as clumsy or
random as a regex; an elegant tool, for a more civilized age."  :)

[1] http://code.google.com/p/error-prone/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread jat

LGTM

Can we deprecate Element.setInnerHTML in favor of setInnerSafeHtml?
Ideally, we would be able to remove the unsafe versions before long.

http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors


[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread t . broyer

On 2012/10/19 12:50:40, tbroyer wrote:

Except you missed one that spans 2 lines in ImageLoadingCell.
(caught using "grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/

|grep

asString |grep -v setInnerHTML" ;-) )


Ah, missed it in the patch; you already caught it, great!

http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors


[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-19 Thread t . broyer

D'oh, where was I when Element#setInnerSafeHtml was added? Didn't know
it even existed!

LGTM++

Except you missed one that spans 2 lines in ImageLoadingCell.
(caught using "grep -R -A 1 'setInnerHTML' user/src/com/google/gwt/
|grep asString |grep -v setInnerHTML" ;-) )

http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors


[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)

2012-10-18 Thread skybrian

LGTM


http://gwt-code-reviews.appspot.com/1857803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors