Last modified: 2014-07-09 21:03:14 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T57582, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 55582 - <bdo>, <q> and other elements are accepted in wikitext, but their attributes are not
<bdo>, <q> and other elements are accepted in wikitext, but their attributes ...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
1.22.0
All All
: Low normal (vote)
: 1.23.0 release
Assigned To: entlinkt
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-10 19:33 UTC by entlinkt
Modified: 2014-07-09 21:03 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Proposed additions to and removals from the attribute whitelist (3.71 KB, patch)
2013-10-11 20:04 UTC, entlinkt
Details

Description entlinkt 2013-10-10 19:33:45 UTC
includes/Sanitizer.php currently includes, in lines 1520/1521:

# 8.2.4
# bdo

Maybe the intention was to not accept the <bdo> element? But then, in lines 399-403 there is:

  $htmlnest = array( [...], 'bdo' );

The result is that the <bdo> element is accepted, but its "dir" attribute is dropped. This is not logical at all because the whole point of the <bdo> element is to specify a "dir" attribute. It is actually required in HTML5: http://www.whatwg.org/specs/web-apps/current-work/multipage/text-level-semantics.html#the-bdo-element
Comment 1 entlinkt 2013-10-10 19:40:31 UTC
Suggested solution: Change lines 1520/1521 to

# 8.2.4
'bdo' => $common,
Comment 2 entlinkt 2013-10-10 20:31:51 UTC
There is more weird stuff happening. I hope the following is complete:

* <bdo> accepts no attributes; should accept $common
* <br> accepts just "id", "class", "title", "style" and "clear"; should accept $common and "clear" (not allowing some of these is an ancient HTML 4 restriction)
* <q> accepts no attributes; should accept $common and "cite"
* <span> accepts $block; should accept just $common (the difference is <span align="...">, which was never legal and doesn't work)
* <wbr> accepts just "id", "class", "title" and "style"; should accept $common (restriction was apparently copied from <br>)
Comment 3 MZMcBride 2013-10-11 02:36:24 UTC
Patches accepted. :-)  You can read [[mw:Gerrit]] for help.
Comment 4 entlinkt 2013-10-11 20:04:49 UTC
Created attachment 13477 [details]
Proposed additions to and removals from the attribute whitelist

This is completely untested. I have never worked with the PHP code.
Comment 5 entlinkt 2013-10-11 20:07:12 UTC
Bugzilla doesn't show the prose at the beginning of the .diff file, so here it is:

Bug 55582: Put the HTML attribute whitelist closer to HTML5

* Add the global attributes to <bdo> and <q> and add "cite" to <q>. This is to make these elements actually usable: <bdo> needs a "dir" attribute to be useful for anything, and the whole point of <q> compared to hard-coded quotation marks is its support for the "lang" and "cite" attributes.
* Drop the "align" attribute from <span> because it was never standards-compliant and does not work in browsers either, unless one constructs such unlikely things as <span align="center" style="display:block;">.
* Drop the obsolete "char" and "charoff" attributes from <tr>, <td>, <th>. These have not been implemented in browsers anyway.
* Drop the obsolete presentational attributes "align", "valign" and "width" from <colgroup>, <col>, <thead>, <tfoot> and <tbody>. These elements are currently not accepted in wikitext anyway, but removing these attributes from the whitelist ensures that they are not accidentally enabled in the future.
* Drop the obsolete presentational attributes "noshade" and "size" from <hr>. They have been overridden by skin-specific CSS for a long time anyway.
* Allow all global attributes on <br> and <wbr>. Not allowing "dir" and "lang" on <br> was a restriction in HTML 4.01, presumably copied to <wbr>, that has been lifted in HTML5. Allowing these may not be particularly useful, but simplifies the code.
Comment 6 Gerrit Notification Bot 2013-10-12 02:46:23 UTC
Change 89384 had a related patch set uploaded by PleaseStand:
Put the HTML attribute whitelist closer to HTML5

https://gerrit.wikimedia.org/r/89384
Comment 7 Kevin Israel (PleaseStand) 2013-10-12 02:58:42 UTC
Please sign up for developer access on Wikitech:

https://wikitech.wikimedia.org/wiki/Special:UserLogin/signup

You cannot leave comments on Gerrit (our code review system) without first creating an account on that wiki. Signing up for an account will also allow
you to upload new patch sets there.
Comment 8 Gerrit Notification Bot 2013-11-02 14:07:10 UTC
Change 89384 merged by jenkins-bot:
Put the HTML attribute whitelist closer to HTML5

https://gerrit.wikimedia.org/r/89384
Comment 9 Bartosz Dziewoński 2013-11-02 14:07:26 UTC
I merged the patch into master. Please, get a developer account to make everybody's lives easier :)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links