Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(193)

Issue 342943002: Center fixpos <dialog> (Closed)

Created:
6 years, 6 months ago by falken
Modified:
6 years, 6 months ago
Reviewers:
esprehn
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr., rune+blink, zoltan1
Project:
blink
Visibility:
Public.

Description

Center fixed positioned <dialog> Now fixed positioned dialogs with auto 'top' and 'bottom' are centered in the viewport, like abspos dialogs. Previously, the dialog would simply not display. There are two failing tests in fixpos-dialog-layout.html. There is still a bug where a <dialog> with auto 'top' and 'bottom' that was *not* centered (an unusual case) is not displayed. BUG=382594 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176590

Patch Set 1 #

Patch Set 2 : fix test #

Patch Set 3 : rm non-anchored-dialog-positioning-expected.txt #

Total comments: 6

Patch Set 4 : review comments #

Messages

Total messages: 13 (0 generated)
falken
Elliott, please take a look. This fixes the worrisome bug for <dialog>. Sorry for the ...
6 years, 6 months ago (2014-06-19 11:01:02 UTC) #1
esprehn
lgtm, but please fix the tests before landing. Really we should just break those tests ...
6 years, 6 months ago (2014-06-20 01:32:37 UTC) #2
esprehn
On 2014/06/20 at 01:32:37, esprehn wrote: > lgtm, but please fix the tests before landing. ...
6 years, 6 months ago (2014-06-20 01:32:53 UTC) #3
falken
Thanks! Makes sense to me. https://codereview.chromium.org/342943002/diff/40001/LayoutTests/fast/dom/HTMLDialogElement/abspos-dialog-layout.html File LayoutTests/fast/dom/HTMLDialogElement/abspos-dialog-layout.html (right): https://codereview.chromium.org/342943002/diff/40001/LayoutTests/fast/dom/HTMLDialogElement/abspos-dialog-layout.html#newcode3 LayoutTests/fast/dom/HTMLDialogElement/abspos-dialog-layout.html:3: <head> On 2014/06/20 01:32:36, ...
6 years, 6 months ago (2014-06-20 01:49:54 UTC) #4
falken
The CQ bit was checked by falken@chromium.org
6 years, 6 months ago (2014-06-20 01:50:14 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/falken@chromium.org/342943002/60001
6 years, 6 months ago (2014-06-20 01:50:44 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_blink_compile_dbg on tryserver.blink ...
6 years, 6 months ago (2014-06-20 02:40:39 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-20 03:34:58 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/12797)
6 years, 6 months ago (2014-06-20 03:34:59 UTC) #9
falken
The CQ bit was checked by falken@chromium.org
6 years, 6 months ago (2014-06-20 03:42:37 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/falken@chromium.org/342943002/60001
6 years, 6 months ago (2014-06-20 03:43:55 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink ...
6 years, 6 months ago (2014-06-20 04:37:16 UTC) #12
commit-bot: I haz the power
6 years, 6 months ago (2014-06-20 05:37:58 UTC) #13
Message was sent while issue was closed.
Change committed as 176590

Powered by Google App Engine
This is Rietveld 408576698