Re: cvs: pear /Structures_DataGrid/DataGrid/Renderer HTMLTable.php

On 3/3/06, Olivier Guilyardi <ojaiml@nerim.net> wrote: > Hi > > Justin Patrin wrote: > > On 3/1/06, Olivier Guilyardi <olivierg@php.net> wrote: > >> Log: > >> Added comments, related to bogus Bug #6151 "Need to encode URL correctly in > >> HTMLTable renderer" > >> > >>+ * Note: users who want their GET parameters separated by > >>+ * "&amp;" instead of "&" (see Bug #6151) > >>should properly > >>+ * configure the "arg_separator.output" php ini setting */ > >> $url .= http_build_query(array_merge($common, $get)); > > > > I don't think that this is right. http_build_query could be used for > > non-HTML output within the same script and changing the php.ini > > setting to use &amp; instead could break those output formats. Imagine > > a script which uses Structures_DataGrid to output an HTML grid to a > > web page but at the same time outputs a text file which includes a > > link generated with http_build_query(). If the seperator is set to > > &amp; the text file will have an invalid URL. > > Agreed. Everything would be simpler if http_build_query() accepted an additional > $separator argument. > Yes. > > If the output is in HTML (and it is since this is an HTML renderer) > > then it is the job of the renderer to run htmlentities before it > > injects this output into the HTML. Setting an option which causes > > http_build_query to always use &amp; instead of & is not the correct > > solution. There could also possibly be other (X)HTML breaking > > characters within this text that should be escaped (although the > > url-encoding likely will not allow this). > > "Run htmlentities()" ? If arg_separator.output is "&amp;" (and AFAIK it > is the > default on certain systems/hosting providers), then we'll get "&amp;amp;".. > True. > > IMHO this option is akin to the infamous magic_quotes_gpc and should > > be worked around in the same manner. It can be used as the same kind > > of premature escaping mechanism. > > Do we have to provide a "workaround" or is this simply a PHP bug ? Why to > provide workarounds for what can be fixed at its root ? > magic_quotes_gpc is also IMHO a PHP bug. Along with overloading. But that's another conversation. > This is not lazyness from me. I just dont want to bloat the code with spaghetti > workarounds. I want drivers to be clean and as simple as possible. The whole > point of the (almost finished) refactoring is to make the drivers easy to : > > 1 - read > 2 - understand > 3 - extend > I understand that. However, forcing people to change a php.ini option to get correct HTML isn't a great solution as some people can't change those options. And, as I said earlier, it can cause output problems wih other formats if someone were to use this function for another format. I would say that either you shouldn't use this function due to possible inconsistencies or check the option and run htmlentities if the option isn't set right. My point is really that it's the job of the output code to make sure that the HTML is escaped correctly, it shouldn't depend on the user altering an option. But then again, maybe I'm completely off-base here. You're welcome to leave it however you want... -- Justin Patrin