From: Bharat M. <bh...@me...> - 2009-08-31 18:44:02
|
Andy Staudacher wrote: > On Mon, Aug 31, 2009 at 11:26 AM, Bharat Mediratta <bh...@me... > <mailto:bh...@me...>> wrote: > > Andy Staudacher wrote: > > On Sun, Aug 30, 2009 at 9:07 PM, Bharat Mediratta > <bh...@me... <mailto:bh...@me...> > > <mailto:bh...@me... <mailto:bh...@me...>>> wrote: > > > > > > Great work! Some comments... > > > > > > Changes: > > p::clean() -> html::clean() > > p::purify() -> html::purify() > > > > > > These seem fine. > > > > > > use html::clean() in views on all PHP output with the > exception > > of... > > - using html::purify() for PHP variables which can contain > > HTML (e.g. comments, item titles, etc.) > > - t() and t2() don't need to be cleaned. The returned > string > > is always clean HTML. > > > > > > Does this mean that we expect the inputs to t() to be clean > already? > > What if I do: > > > > <?= t("%foo", array("foo" => "<html>")) ?> > > > > Will that return "<html>" or "<html>"? > > > > > > The latter. If you do <?= t("<span>%foo</span>", array("foo" => > > "<html>")) ?> > > it will return "<span><html></span>". > > > > If you have parameters to t() that are clean already, do: > > <?= t("<span>%foo</span>", array("foo" => > html::mark_safe("<html>"))) ?> > > and it will return "<span><html></span>". > > > > use t() and t2() in JavaScript as: var js_var = <?= > > t("Hello")->for_js() ?>; > > > > escape PHP vars for JavaScript: var js_var = <?= > > html::js_string($some_php_string) ?>; > > > > escape PHP vars for HTML element attributes: <a title="<?= > > html::clean_attribute($php_string) ?>"> > > > > > > These names seem inconsistent. Can you make the first one > > html::clean_js() so that they're all clean_xxx() methods? OR am I > > missing something? > > > > > > Already considered and rejected. js_string() is indeed not the same as > > clean() just for js. > > clean() implies the returned value is safe for inclusion in HTML. > > clean_js() would imply that the returned value is safe for use in JS, > > anywhere. > > js_string() is very descriptive, yet short. It returns a JS string. > > I understand now from looking at the code. So before we had: > > var x = "<?= $some_string ?>"; > > Now we have: > > var x = <?= html::js_string($some_string) ?>; > > I recognize that you're adding the outer quotation marks as a > convenience in the js_string() function, but I worry that this will be a > little confusing to JS developers who are expecting strings to be inside > quotation marks. Perhaps we could have it do: > > > Minor tidbit: It's not just convenience to include the delimiting > quotation marks. It's about semantics. What is returned is a JavaScript > string object. > > More importantly, there's no way (without tons of effort) I can > guarantee that some string is "clean JavaScript." What's clean (purified > / harmless) JavaScript anyway? > I can just guarantee that the returned string is safe to use as a > JavaScript string. That the returned string won't add any active code to > your JavaScript (unless you use the JavaScript string later as event > handler or the like). > > So returning a complete JavaScript string object is very clear IMO. Just > like json_encode(). I agree that to some developers this will be clear. But I'm betting we're going to see a lot of people using this API wrong because they simply won't expect that quotation marks are added automatically. They're going to add a second set of quotes around it, and that's going to cause weird errors. > > - Andy > > var x = "<?= html::clean_js_string($some_string) ?>"; > > That's longer, but it's got "clean" in it as well as "js_string" and a > JS developer would see the outer quotes so would know that anything > inside it is definitely in a JS string context. > > thoughts? > > Everything else looks fine to me. > > > > > clean_attribute() is another story. It somewhere similar to clean(), > > just specific for attributes. > > > > when passing a URL to t() and t2() use html::mark_safe(): > > <span><?= t('Click <a href="%url">here</a>', array("url" => > > html::mark_safe(url::site("foo")))) ?></span> > > > > > > And here, can we make it html::mark_clean() so that clean is > in the > > API somewhere? > > > > > > OK, makes sense. Will do. > > > > - Andy > > > > > > -Bharat > > > > > > |