[gwt-contrib] Re: Replace instances of element.setInnerHTML(safeHtml.asString()) (issue1857803)
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)
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)
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)
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)
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)
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)
LGTM http://gwt-code-reviews.appspot.com/1857803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors