| Summary: | [Qt] tab-size CSS property doesn't work at all | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||
| Component: | Tools / Tests | Assignee: | Bruno Abinader (history only) <bruno.abinader> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | allan.jensen, bruno.abinader, cmarcelo, morrita, ossy, simon.fraser, webkit.review.bot | ||||||
| Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Bug Depends on: | 85203, 92119 | ||||||||
| Bug Blocks: | 52994, 87008 | ||||||||
| Attachments: |
|
||||||||
|
Description
Csaba Osztrogonác
2012-05-11 00:04:23 PDT
I skipped it on Qt until proper fix - https://trac.webkit.org/changeset/116734 It is not limited to tab-size. Qt platform doesn't support tabs at all. We always normalize the tabs to spaces before drawing them. Adding bug 85203 as dependency, using Caio's patches for modifying the default font the results are now passing. The issue is related to the fact that non-monospaced fonts do have different lengths for each character, so now if the tab size, for example, equals to 8, then it will be exactly the lenght of a whitespace multiplied by 8. For even better control,a font-family:monospace would force even more proper image comparison results, what do you guys think? Created attachment 147619 [details] "Force tab size layout test to use monospace" patch This patch forces the tab size layout test to use monospace font family. However, please notice this patch *still* does not fix the Qt issue as it depends on bug 85203. This patch makes sure every other platform uses monospace to avoid character/whitespace length variations that could cause test image comparison to fail. (In reply to comment #2) > It is not limited to tab-size. Qt platform doesn't support tabs at all. We always normalize the tabs to spaces before drawing them. This comment is obviously wrong, so just ignore it. I was confused by the fact that some of other backends handle tabs in the platform-code while Qt doesn't. Comment on attachment 147619 [details] "Force tab size layout test to use monospace" patch View in context: https://bugs.webkit.org/attachment.cgi?id=147619&action=review > LayoutTests/fast/css/tab-size.html:43 > +<body style="font-family:'monospace'" onload="test();"> > <div id="template" style="display:none;"> > <pre>	x</pre> Setting the style here is wrong. We are using <pre> tag and it should select a monospace font in the test environment. Thanks Caio :) My bad actually, I've runned the patch back when patch from bug 85203 wasn't applied yet, so <pre> was not properly using monospace-like fonts, thus the forced usage. After bug 85203, layout tests results are okay, so if anyone mind I'm setting this bug to "fixed". (In reply to comment #6) > (From update of attachment 147619 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147619&action=review > > > LayoutTests/fast/css/tab-size.html:43 > > +<body style="font-family:'monospace'" onload="test();"> > > <div id="template" style="display:none;"> > > <pre>	x</pre> > > Setting the style here is wrong. We are using <pre> tag and it should select a monospace font in the test environment. Bruno, could you make a patch to unskip it? :-) Created attachment 154032 [details] Unskip layout test patch This patch unskips the fast/css/tab-size.html layout test, after monospace fix from r121971. Marking as "reopened" to trigger commit-queue. Comment on attachment 154032 [details] Unskip layout test patch Clearing flags on attachment: 154032 Committed r123478: <http://trac.webkit.org/changeset/123478> All reviewed patches have been landed. Closing bug. |