Page MenuHomePhabricator

<br clear=(all|left|right|none)> breaks XHTML compilance
Closed, DeclinedPublic

Assigned To
Authored By
bzimport
Mar 8 2005, 6:24 PM
Referenced Files
F1875: fixtags.php
Nov 21 2014, 8:14 PM
F1874: Parser.php.patch
Nov 21 2014, 8:14 PM
F1873: Parser.php.patch
Nov 21 2014, 8:14 PM

Description

Author: avarab

Description:
using <br clear=all> breaks XHTML compilance. This patch fixes the issue.


Version: 1.5.x
Severity: blocker
Platform: Other

Details

Reference
bz1661
TitleReferenceAuthorSource BranchDest Branch
Parse html to a DOM objectrepos/content-transform/pagesummary!15febinbellamyT366196main
common-impact-analytics: Some blubber updatesrepos/generated-data-platform/aqs/commons-impact-analytics!4sfaciT366156-update-blubbermain
Added double-submit cookie and some other login related improvementsrepos/data-engineering/mpic!32sfaciT365182-double-submit-cookiemain
drop_gu_salt_T366123.py: Needs to use centralauthrepos/sre/schema-changes!18marosteguiT366123main
Change the datahub ingestion urlrepos/data-engineering/airflow-dags!710stevemunenechange_datahub_gms_endpointmain
drop_gu_salt_T366123.py: New schema changerepos/sre/schema-changes!17marosteguiT366123main
Customize query in GitLab

Revisions and Commits

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 8:14 PM
bzimport set Reference to bz1661.
bzimport added a subscriber: Unknown Object (MLST).

avarab wrote:

The patch against includes/Parser.php

A patch for HEAD.

Attached:

avarab wrote:

The patch against includes/Parser.php (for REL1_4)

The patch against REL1_4.

Attached:

avarab wrote:

A testcase for the regular expression

A testcase for the expression.

Attached:

Several things wrong with this patch:

First, it does not appear to be notice-clean. With error_reporting set to E_ALL,
notices will be displayed about the use of undefined constants.

Second, it will fail to pick up any <br> containing additional attributes (for
instance, an id or a style attribute) so it is insufficient.

Third, it gratuitously transforms the clear attribute to an inline style. This is
unnecessary for our XHTML 1.0 Transitional target. (If it were necessary, it would
also be necessary to be able to merge it with a specified style attribute if present.)

Consider instead working on Sanitizer.php to have the HTML sanitizer output the
/> automatically on manditorily-closed empty elements.

avarab wrote:

  1. It isn't, I didn't test it for that.
  1. No it won't, it was designed for this specific case and you're right, it's

insufficient.

  1. This is what currently happens if you type <br clear="both"/>, the wiki will

render <br style="clear: both;" />, however if you type <br clear="all" /> or
<br clear="all"> it will output what you gave it.

  1. Sanitizer.php is probably better, any suggestions as to where to start?
epriestley changed the task status from Declined to Resolved by committing Unknown Object (Diffusion Commit).Mar 4 2015, 8:23 AM
epriestley added a commit: Unknown Object (Diffusion Commit).
Aklapper changed the task status from Resolved to Declined.Mar 4 2015, 11:48 AM
Aklapper claimed this task.