php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77348 Misleading Edit button (edition not immediate and conditional to application)
Submitted: 2018-12-26 18:29 UTC Modified: 2019-01-02 17:18 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: chealer at gmail dot com Assigned:
Status: Open Package: Online Doc Editor problem
PHP Version: Irrelevant OS:
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2018-12-26 18:29 UTC] chealer at gmail dot com
Description:
------------
I noticed tens of issues in PHP documentation over the years. Sometimes, I tried to get them fixed. I opened bug reports, and about 5 times I must have tried to use "Php Docbook Online Editor".

Unfortunately, to my knowledge none of my attempts to fix the doc with Php Docbook Online Editor worked. I had such a strong suspicion that the last 2 attempts, I noted what I did to confirm.

As reported in http://news.php.net/php.doc/969385815 I submitted a proposal in 2015 for addslashes which was not accepted. It appears that this proposal is completely lost. I cannot even find any evidence that it was made. I reported this to the phpdoc mailing list years ago and did not get any answer:
http://news.php.net/php.doc/969385815

Some time later, as I did not trust Php Docbook Online Editor, I submitted issue report #70120: https://bugs.php.net/bug.php?id=70120&edit=2
Unfortunately, the page still has several issues, so I tried to use Php Docbook Online Editor once again to actually solve the issue on 2016-03-26, but that time I took notes. And unfortunately, over 2 years later, my proposal is neither accepted, nor even acknowledged in any way that I can see. I did not receive any email, which is not surprising given that I was not asked for an address.

So once again, the Edit button which seems to suggest the documentation is a wiki misled me. I thought I could edit easily, but in fact I put my time in the garbage.

I am guessing that Php Docbook Online Editor is not completely broken, so I am not going to suggest dismantlement, but at least one of the following should be done:
1. Make it less likely that contributors reach Php Docbook Online Editor from the documentation. Either remove the link, move it somewhere less prominent, or change the label ("Propose a change / Edit" would already be less misleading).
2. Implement basic features and do minimal quality assurance to confirm that these features work.
3. Add a warning when launching Php Docbook Online Editor that it is experimental, or - better yet - list the main areas known to be broken.


Patches

pdo-prepare (last revision 2018-12-27 11:32 UTC by cmb@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-12-26 18:39 UTC] chealer at gmail dot com
After I submitted a proposal for prepare(), the Patches for review tab indicated "(0)". Yet, it contained stuff from more than 20 people.

The contents of my proposal follow:
<?xml version="1.0" encoding="utf-8"?>
<!-- $Revision: 337261 $ -->
<refentry xml:id="pdo.prepare" xmlns="http://docbook.org/ns/docbook">
 <refnamediv>
  <refname>PDO::prepare</refname>
  <refpurpose>
   Prepares a statement for execution and returns a statement object
  </refpurpose>
 </refnamediv>
 <refsect1 role="description">
  &reftitle.description;
  <methodsynopsis>
   <modifier>public</modifier> <type>PDOStatement</type><methodname>PDO::prepare</methodname>
   <methodparam><type>string</type><parameter>statement</parameter></methodparam>
   <methodparam choice="opt"><type>array</type><parameter>driver_options</parameter><initializer>array()</initializer></methodparam>
  </methodsynopsis>

  <para>
   Prepares an SQL statement template to be executed by the
   <function>PDOStatement::execute</function> method. The statement template can
   contain zero or more named (:name) or question mark (?) parameter markers
   for which real values will be substituted when the statement template is executed.
   Both named and question mark parameter markers cannot be used within the same
   SQL statement template; only one or the other parameter style.
   Use these parameters to bind any user-input, do not include the user-input
   directly in the query.
  </para>
  <para>
   You must include a unique parameter marker for each value you wish to pass
   in to the statement when you call <function>PDOStatement::execute</function>.
   You cannot use a named parameter marker of the same name more than once in a prepared
   statement, unless emulation mode is on.
  </para>
  <note>
   <para>
    Parameter markers can represent a complete data literal only.
    Neither part of literal, nor keyword, nor identifier, nor whatever arbitrary query 
    part can be bound using parameters. For example, you cannot bind multiple values 
    to a single parameter in the IN() clause of an SQL statement.
   </para>
  </note>
  <para>
   Calling <function>PDO::prepare</function> and
   <function>PDOStatement::execute</function> for statements that will be
   issued multiple times with different parameter values optimizes the
   performance of your application by allowing the driver to negotiate
   client and/or server-side caching of the query plan and meta information. Also, calling <function>PDO::prepare</function> and
   <function>PDOStatement::execute</function> helps to prevent SQL injection attacks by eliminating the need to
   manually quote and escape the parameters.
  </para>
  <para>
   PDO will emulate prepared statements/bound parameters for drivers that do
   not natively support them, and can also rewrite named or question mark
   style parameter markers to something more appropriate, if the driver
   supports one style but not the other.
  </para>
 </refsect1>
 <refsect1 role="parameters">
  &reftitle.parameters;
  <para>
   <variablelist>
    <varlistentry>
     <term><parameter>statement</parameter></term>
     <listitem>
      <para>
       This must be a valid SQL statement template for the target database server.
      </para>
     </listitem>
    </varlistentry>
    <varlistentry>
     <term><parameter>driver_options</parameter></term>
     <listitem>
      <para>
       This array holds one or more key=&gt;value pairs to set
       attribute values for the PDOStatement object that this method
       returns. You would most commonly use this to set the
       <literal>PDO::ATTR_CURSOR</literal> value to
       <literal>PDO::CURSOR_SCROLL</literal> to request a scrollable cursor.
       Some drivers have driver-specific options that may be set at
       prepare-time.
      </para>
     </listitem>
    </varlistentry>
   </variablelist>
  </para>
 </refsect1>

 <refsect1 role="returnvalues">
  &reftitle.returnvalues;
  <para>
   If the database server successfully prepares the statement,
   <function>PDO::prepare</function> returns a
   <classname>PDOStatement</classname> object.
   If the database server cannot successfully prepare the statement,
   <function>PDO::prepare</function> returns &false; or emits
   <classname>PDOException</classname> (depending on <link
   linkend="pdo.error-handling">error handling</link>).
  </para>
  <note>
   <para>
    Emulated prepared statements does not communicate with the database server
    so <function>PDO::prepare</function> does not check the statement.
   </para>
  </note>
 </refsect1>

 <refsect1 role="examples">
  &reftitle.examples;
  <para>
   <example><title>Prepare an SQL statement template with named parameters</title>
    <programlisting role="php">
<![CDATA[
<?php
/* Execute a prepared statement by passing an array of values */
$sql = 'SELECT name, colour, calories
    FROM fruit
    WHERE calories < :calories AND colour = :colour';
$sth = $dbh->prepare($sql, array(PDO::ATTR_CURSOR => PDO::CURSOR_FWDONLY));
$sth->execute(array(':calories' => 150, ':colour' => 'red'));
$red = $sth->fetchAll();
$sth->execute(array(':calories' => 175, ':colour' => 'yellow'));
$yellow = $sth->fetchAll();
?>
]]>
    </programlisting>
   </example>
   <example>
    <title>Prepare an SQL statement template with question mark parameters</title>
    <programlisting role="php">
<![CDATA[
<?php
/* Execute a prepared statement by passing an array of values */
$sth = $dbh->prepare('SELECT name, colour, calories
    FROM fruit
    WHERE calories < ? AND colour = ?');
$sth->execute(array(150, 'red'));
$red = $sth->fetchAll();
$sth->execute(array(175, 'yellow'));
$yellow = $sth->fetchAll();
?>
]]>
    </programlisting>
   </example>
  </para>
 </refsect1>

 <refsect1 role="seealso">
  &reftitle.seealso;
  <para>
   <simplelist>
    <member><function>PDO::exec</function></member>
    <member><function>PDO::query</function></member>
    <member><function>PDOStatement::execute</function></member>
   </simplelist>
  </para>
 </refsect1>
</refentry>

<!-- Keep this comment at the end of the file
Local variables:
mode: sgml
sgml-omittag:t
sgml-shorttag:t
sgml-minimize-attributes:nil
sgml-always-quote-attributes:t
sgml-indent-step:1
sgml-indent-data:t
indent-tabs-mode:nil
sgml-parent-document:nil
sgml-default-dtd-file:"~/.phpdoc/manual.ced"
sgml-exposed-tags:nil
sgml-local-catalogs:nil
sgml-local-ecat-files:nil
End:
vim600: syn=xml fen fdm=syntax fdl=2 si
vim: et tw=78 syn=sgml
vi: ts=1 sw=1
-->
 [2018-12-26 23:38 UTC] cmb@php.net
I suppose these patches have not been swallowed by PhDOE, but
rather have been rejected, maybe because they have gone stale
(PdDOE apparently doesn't store diffs, but rather the complete
file, so if someone else edits the file, the presented patch often
makes no sense anymore).  Also, we likely delete unsuitable
patches without any notice to submitters, since often it is not
possible to notify them.

> Make it less likely that contributors reach Php Docbook Online
> Editor from the documentation. Either remove the link, move it
> somewhere less prominent, or change the label ("Propose a change /
> Edit" would already be less misleading).

I think that we should remove the link altogether.  A considerable
percentage of submitted patches are done by trolls who try to make
malicious or at least obviously bad changes.  It's rather annoying
to check and delete these patches (if the latter is even allowed).

In the (hopefully not too) long run we should really switch to
Git[1] to be able to replace PhDOE with Github pull requests.

> After I submitted a proposal for prepare(), the Patches for
> review tab indicated "(0)". Yet, it contained stuff from more than
> 20 people.

Indeed, the count appears to be broken.  However, I can't find
your patch regarding PDO::prepare, and there are currently only
submitted patches from eight people for the *English* language.

[1] <http://news.php.net/php.doc/969386622>
 [2018-12-27 04:06 UTC] chealer at gmail dot com
Thank you for checking cmb. The last proposal I made was in 2016, so it is expected that it doesn't show in the Patches for review tab. The question is where it shows.

I am not saying that reviewers are mishandling proposals, but there is something wrong in the system as a whole (the engine plus its operators) if the end result is that people who logged in and did not refuse to provide any contact information end up not being notified of rejections. And really wrong if even by returning to the website they can't figure out who rejected, when or why.
 [2018-12-27 11:32 UTC] cmb@php.net
The following patch has been added/updated:

Patch Name: pdo-prepare
Revision:   1545910369
URL:        https://bugs.php.net/patch-display.php?bug=77348&patch=pdo-prepare&revision=1545910369
 [2018-12-27 11:32 UTC] cmb@php.net
> The last proposal I made was in 2016, […]

Ah, I see.  Basically, I think the patch is fine, but I would use
“template” only when referring to the $statement parameter. See
the attached patch pdo-prepare.

> The question is where it shows.

I'm afraid, nowhere.

> […] but there is something wrong in the system as a whole (the
> engine plus its operators) […]

ACK.  However, I don't think we should spend much time on
improving PhDOE, given that there are way more suitable
alternatives *almost* readily available, such as Github pull
requests.  Personally, I even prefer to have a bug report with an
attached patch instead of a submission to PhDOE, unless it's about
a simple typo fix or such.  While this is slightly more work, it
allows to discuss the patches and to track the progress.
 [2018-12-27 14:10 UTC] cmb@php.net
Back to the issue at hand: the relevant line that would have to be
changed or removed is
<https://github.com/php/web-php/blob/bda2d837724d59efe8580b9232d6d60fa545cf5b/include/shared-manual.inc#L448>
 [2019-01-02 06:13 UTC] chealer at gmail dot com
-Summary: Misleading Edit button (Experimental but not flagged as such) +Summary: Misleading Edit button (edition not immediate)
 [2019-01-02 06:13 UTC] chealer at gmail dot com
Thanks to much luck, just after reporting this I was pointed to a tutorial about using Php Docbook Online Editor, which explains the step I must have been missing: https://www.youtube.com/watch?v=HLAuzZh2GVo
This tutorial is in French.

After, I completed each step to submit a patch against PDO::prepare(). I can now see that my changes are reflected in the actual documentation:
http://svn.php.net/viewvc/phpdoc/en/trunk/reference/pdo/pdo/prepare.xml?r1=337261&r2=346459

This process still appears to be limited as the submitter's name and change description appear to be lost, but given that it works and is quite straightforward, I apologize for describing the tool as experimental. I would rather argue that offering a save button (with the floppy disk icon) combined with the label "Edit" and without instructions when a contributor saves is likely to mislead, but that's a much less severe issue.

I recommend to:
1. Relabel "Edit" to "Propose a change / Edit"
2. Make Php Docbook Online Editor's behavior or interface reflect that the edition process has 2 phases. This could be done by either warning each user on their first use, or by requiring users to perform a "Start a patch" action before editing files, so they realize what they are editing are temporary files only meant to generate a patch.
 [2019-01-02 13:59 UTC] cmb@php.net
> After, I completed each step to submit a patch against
> PDO::prepare(). I can now see that my changes are reflected in the
> actual documentation:

In practise, it is not really relevant whether a patch is listed
as work in progress, or whether it has been submitted as patch for
review.  In both cases somebody with sufficient karma will have to
review the patch, and commit or reject it.
 [2019-01-02 17:18 UTC] chealer at gmail dot com
-Summary: Misleading Edit button (edition not immediate) +Summary: Misleading Edit button (edition not immediate and conditional to application)
 [2019-01-02 17:18 UTC] chealer at gmail dot com
Thank you cmb
In that case, I suppose there is an issue causing patch creators to get no notification when their patch is rejected, and to have no way to understand the rejection even if they access Php Docbook Online Editor again.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Wed Jan 16 12:01:25 2019 UTC