CssSchema: Allow negative token values for margin property #184
Conversation
"top", "bottom", "left", "right" and "margin" are allowed to be negative. "width" and "height" type properties should always be positive.
|
How could my fix possibly break the build system? Thanks, |
@janis-github The baseline run in Travis isn't super old, but sometimes this happens when Travis has changed something like the set of supported JDKs, or a new version of the maven test coverage plugin does things subtly differently. If we resolve other issues, I'm happy to pull and merge manually. |
|
Thanks for the patch. There used to be a problem with negative margins allowing HTML like the below to escape visual containment mechanisms like CSS <!doctype html>
<html>
<meta charset="utf-8" />
<body>
<!-- trusted content -->
<form style="float: right; width: 10em" id="sidebar">
<label for="account">Email:</label> <input type="text" name="account">
<br><label for="pwd">Password:</label> <input type="password" name="pwd"></p>
</form>
<div style="width: 30em">
<p>Lorem ipsum sic dolor amet
<p>Lorem ipsum sic dolor amet
<p>Lorem ipsum sic dolor amet
<p>Lorem ipsum sic dolor amet
<h2>Comments</h2>
<div style="overflow: auto">
<!-- /trusted content -->
<!-- sanitized, third-party content -->
<p>Comment content</p>
<p style="margin: -8em 0 8em 25em">Contact user support +1-800-PHISHER</p>
<!-- /sanitized, third-party content -->
</div>
</html>It used to be the case, that certain browsers (sorry I forget which) used to render that HTML thus:
Note that the "Contact user support +1-800-PHISHER" text appears below the password input where it seems to be part of the high-reputation login form, instead of being constrained to the rectangle for low-reputation content. Before allowing negative margins, I'd want to double check that browsers no longer have that bug. Would replacing negative values with |
|
I see your point. I was thinking about that and similar behaviour can be achieved with large positive margins going out of the box, too. (Do not have sample for that, though) There is no solution based only on margin values. So, normalising negatives to 0 (or any other number for that matter) does not solve the problem. In my specific case which triggered this issue setting left margin to -1.6em did actually make the output prettier. |
|
@janis-github, Sorry for the delay. The problem was that visual cropping was bypassed when margins were negative. Not that you can move things around. I'm not aware of any numeric overflow problems that bypassed visual cropping with large margins. |
|
Ok, resetting negative margins to 0 probably is the best solution then. Sorry, I'll not have a PR as it involves going significantly deeper in your code as I'd like, currently. I.e. resetting token values to zero instead of just ignoring. |
This fixes the #183