Summary: | Float block's logical top margin is illegal in vertical writing mode. | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yuki Sekiguchi <yuki.sekiguchi> | ||||||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | dglazkov, eric, morrita, noel.gordon, ojan.autocc, webkit.review.bot | ||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||
Attachments: |
|
Created attachment 163768 [details]
Patch
Comment on attachment 163768 [details] Patch Attachment 163768 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13832638 New failing tests: fast/multicol/vertical-rl/float-multicol.html Created attachment 164017 [details]
Add need rebaseline for chromium.
fast/multicol/vertical-rl/float-multicol.html regression is intentional. I watch it on Chromium Linux and it works well. I add need rebaseline for Chromium. Created attachment 164050 [details]
Rebase
Comment on attachment 164050 [details] Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=164050&action=review Looks good. Please adress my nitpicky comment. > Source/WebCore/ChangeLog:8 > + Float block's logical top margin is illegal in vertical writing mode. Please don't repeat this. > LayoutTests/ChangeLog:9 > + This test's block should be at the center of black border This explanation should be in the test so that test is more readable. Created attachment 171766 [details]
Fix test indications
Thank you for reviewing, Morrita-san! (In reply to comment #6) > > Source/WebCore/ChangeLog:8 > > + Float block's logical top margin is illegal in vertical writing mode. > > Please don't repeat this. Removed. > > LayoutTests/ChangeLog:9 > > + This test's block should be at the center of black border > > This explanation should be in the test so that test is more readable. Moved this description to test. I changed "inside" to "at the center of" in vertical-float-margin.html. Comment on attachment 171766 [details] Fix test indications Rejecting attachment 171766 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: atching file LayoutTests/fast/writing-mode/vertical-float-margin-expected.html patching file LayoutTests/fast/writing-mode/vertical-float-margin.html patching file LayoutTests/platform/chromium/TestExpectations Hunk #1 FAILED at 4162. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/TestExpectations.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Hajime Mor..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/14717669 Created attachment 172896 [details]
Rebase
Created attachment 178672 [details]
Rebase
Created attachment 178687 [details]
Rebase
Comment on attachment 178687 [details] Rebase Attachment 178687 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15257440 New failing tests: fast/text/emphasis-vertical.html Created attachment 178943 [details]
Fix TestExpectations to need rebaseline
Morrita-san, could you review it? Rebasing for commit queue make a chromium test fail. I add it to TestExpectations. Created attachment 181617 [details]
Patch
Morrita-san, could you review it? You give lgtm to this patch. However, rebasing for commit queue make a chromium test fail. I add it to TestExpectations. Comment on attachment 181617 [details]
Patch
Sure.
Just FYI...
You can upload patch with
- replacing NOBODY by reviewers name,
- leaving r field empty and
- just set cq field.
Doing it tells that the patch is already reviewed and ready to commit.
Any committer (not reviewer) can check it in.
In #webkit IRC channel, it is pretty normal to ask to commit such patches.
Thank you for your advice! I try it at next time that commit queue is rejected. Comment on attachment 181617 [details] Patch Clearing flags on attachment: 181617 Committed r139040: <http://trac.webkit.org/changeset/139040> All reviewed patches have been landed. Closing bug. There are two entries for fast/text/emphasis-vertical.html in TestExpectations: # Needs rebaseline. Current expectation is wrong because margin of body don't collapse with margin of 1st child. webkit.org/b/96597 fast/text/emphasis-vertical.html [ ImageOnlyFailure ] # WebKit roll 74255:74308 crbug.com/67416 [ Win Mac Android ] fast/text/emphasis-vertical.html [ Failure ImageOnlyFailure ] one overrides the other. Do you intend to fix? From http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/32571/steps/layout-test/logs/stdio Parsing expectations ... --lint-test-files warnings: ... LayoutTests\platform\chromium\TestExpectations:4364 More specific entry for fast/text/emphasis-vertical.html on line LayoutTests\platform\chromium\TestExpectations:4364 overrides line LayoutTests\platform\chromium\TestExpectations:2589. fast/text/emphasis-vertical.html ... (In reply to comment #22) > There are two entries for fast/text/emphasis-vertical.html in TestExpectations: > > # Needs rebaseline. Current expectation is wrong because margin of body don't collapse with margin of 1st child. > webkit.org/b/96597 fast/text/emphasis-vertical.html [ ImageOnlyFailure ] > # WebKit roll 74255:74308 > crbug.com/67416 [ Win Mac Android ] fast/text/emphasis-vertical.html [ Failure ImageOnlyFailure ] > > one overrides the other. Do you intend to fix? Sorry for late reply. I missed this comment. Stephen fixed this at r139467: http://trac.webkit.org/changeset/139467 Thank you, Stephen. |
Created attachment 163767 [details] reproduced content Float block's logical top margin is illegal in vertical writing mode. This bug is reproduced by the attached HTML content. Blue block have offset to left although the style is margin: 50px. The block should be at the center of black border.