New issue
Advanced search Search tips

Issue 835279 link

Starred by 3 users
Status: Fixed
Owner:
steimel@chromium.org
Closed: Apr 2018
Cc:
nyerramilli@chromium.org
rbasuvula@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-04-30
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression


Show other hotlists

Hotlists containing this issue:
Modern-Media-Controls


Sign in to add a comment

Regression: Download option is seen misplaced on https://tidbits.com/podcasts/14397.m4a

Reported by aiman.an...@etouch.net, Apr 20 2018 
Chrome Version: 68.0.3401.0 (Official Build)Revision d258c5155b1bb1ca893ee6f86345c341bb6d98f4-refs/heads/master@{#552221}(32/64-bit).

OS: Windows(7,8,8.1,10), Mac(10,12,6, 10.13.1, 10.13.5), Linux(14.04 LTS).

Test URL: https://tidbits.com/podcasts/14397.m4a

What steps will reproduce the problem?
1. Launch chrome, navigate to the above URL and click on 3-Dot icon.
2. Observe

Actual: Download option is seen misplaced (seen in top left corner).
Expected: Download option should not be seen misplaced.

This is a regression issue, broken in M-68 series, and providing suspect via change-log.
(Unable to provide bisect using per-revision script as builds not getting generated and also using old bisect script as url is not accessible in chromium builds)

Good Build:68.0.3400.0
Bad Build:68.0.3401.0

Change-log URL:
https://chromium.googlesource.com/chromium/src/+log/68.0.3400.0..68.0.3401.0?pretty=fuller&n=10000

Suspecting: r551926?

pkasting@: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You!
Actual Result.mov
2.3 MB View Download
Expected Result.mov
2.1 MB View Download

Comment 1 by cbiesin...@chromium.org, Apr 24 2018

Components: -Blink Blink>Layout
Owner: steimel@chromium.org
Not likely to be pkasting. Possibly Tommy in https://chromium-review.googlesource.com/1005508 ?

Comment 2 by steimel@chromium.org, Apr 25 2018 

Issue 836714 has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 26 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a47ca7b38e282eed67402d110810f132302cd1a

commit 3a47ca7b38e282eed67402d110810f132302cd1a
Author: Tommy Steimel <steimel@chromium.org>
Date: Thu Apr 26 19:18:57 2018

Remove audio element centering on MediaDocument

This CL removes a CSS rule that centered the audio element on the
MediaDocument page. This fixes an issue where the overflow menu position
calculation was incorrect.

Bug:  835279 
Change-Id: Ib957338e0e7d3d5074a93fb1e89e52a032987e4b
Reviewed-on: https://chromium-review.googlesource.com/1028483
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554112}
[delete] https://crrev.com/5a3f59b5075e6f0993488d65dbf6e5b7c8b40898/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.png
[delete] https://crrev.com/5a3f59b5075e6f0993488d65dbf6e5b7c8b40898/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/mac-mac10.12/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.txt
[delete] https://crrev.com/5a3f59b5075e6f0993488d65dbf6e5b7c8b40898/third_party/WebKit/LayoutTests/platform/win7/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/3a47ca7b38e282eed67402d110810f132302cd1a/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css

Comment 4 by steimel@chromium.org, Apr 26 2018

Labels: -M-68 -RegressedIn-68 -Target-68 RegressedIn-67 Merge-Request-67 M-67 Target-67
Status: Fixed (was: Assigned)

Comment 5 by gov...@chromium.org, Apr 26 2018 

Before we approve merge to M67, please answer followings:
* Is this M67 regression? Is it critical?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M67?
* Any other important details to justify the merge.

Please note M67 is already in Beta, so merge bar is very high. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 27 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e1a9b9670201871772c142542005eb6daed3fe7

commit 5e1a9b9670201871772c142542005eb6daed3fe7
Author: Sam McNally <sammc@chromium.org>
Date: Fri Apr 27 01:23:55 2018

Revert "Remove audio element centering on MediaDocument"

This reverts commit 3a47ca7b38e282eed67402d110810f132302cd1a.

Reason for revert: Failing on WebKit Win10: https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Win10/34081

Original change's description:
> Remove audio element centering on MediaDocument
> 
> This CL removes a CSS rule that centered the audio element on the
> MediaDocument page. This fixes an issue where the overflow menu position
> calculation was incorrect.
> 
> Bug:  835279 
> Change-Id: Ib957338e0e7d3d5074a93fb1e89e52a032987e4b
> Reviewed-on: https://chromium-review.googlesource.com/1028483
> Commit-Queue: Tommy Steimel <steimel@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554112}

TBR=mlamouri@chromium.org,steimel@chromium.org

Change-Id: Ifada3e4d326ec6872cd124f381003c0ad3d6e62f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  835279 
Reviewed-on: https://chromium-review.googlesource.com/1031830
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554254}
[add] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.png
[add] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/mac-mac10.10/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/mac-mac10.12/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.txt
[add] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/WebKit/LayoutTests/platform/win7/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/5e1a9b9670201871772c142542005eb6daed3fe7/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css

Comment 7 by gov...@chromium.org, Apr 27 2018 

CL listed at #3 is reverted at #6. So there  is no M67 merge needed, correct?
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 28 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c

commit f0b02b2c35400a2cc1bd85094a5f02adf50ca92c
Author: Tommy Steimel <steimel@chromium.org>
Date: Sat Apr 28 01:25:45 2018

Reland remove audio element centering on MediaDocument

This is a reland of crrev.com/c/1028483. This CL removes a CSS rule
that centered the audio element on the MediaDocument page. This fixes
an issue where the overflow menu position calculation was incorrect.

Bug:  835279 
Change-Id: I028383f5201c5bee741a6fb5dd5a825a9a808736
Reviewed-on: https://chromium-review.googlesource.com/1033914
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554599}
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/mac-mac10.10/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/mac-mac10.12/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/WebKit/LayoutTests/platform/win7/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/f0b02b2c35400a2cc1bd85094a5f02adf50ca92c/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 29 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by gov...@chromium.org, Apr 29 2018

NextAction: 2018-04-30
Is CL listed at #8 only need to merge to M67? Also is CL listed at #8 looking good in canary?

Comment 11 by monor...@bugs.chromium.org, Apr 30 2018 

The NextAction date has arrived: 2018-04-30

Comment 12 by aiman.an...@etouch.net, Apr 30 2018 

Hi,

Retested the above issue on latest Canary build #68.0.3415.0 using Windows(7,8,8.1,10), Linux(14.04 LTS) and Mac(10.12.6, 10.13.1, 10.13.5) and issue is fixed now.
Download button is no more misplaced.

Kindly refer screen-cast for reference.

Thank You!
Current_Result.mp4
173 KB View Download

Comment 13 by steimel@chromium.org, Apr 30 2018 

Yes, just the CL in #8 is needed. Looks good in canary for me

Comment 14 by gov...@chromium.org, Apr 30 2018

Labels: -Merge-Review-67 Merge-Approved-67
Approving merge for CL listed at #8 to M67 branch 3396 per comment #13. Please merge ASAP so we can pick it up for this week Beta release. Thank you.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 30 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3999712776f6fd88caa4a882cf64c2a738a9fc9

commit a3999712776f6fd88caa4a882cf64c2a738a9fc9
Author: Tommy Steimel <steimel@chromium.org>
Date: Mon Apr 30 17:31:26 2018

Reland remove audio element centering on MediaDocument

This is a reland of crrev.com/c/1028483. This CL removes a CSS rule
that centered the audio element on the MediaDocument page. This fixes
an issue where the overflow menu position calculation was incorrect.

Bug:  835279 
Change-Id: I028383f5201c5bee741a6fb5dd5a825a9a808736
Reviewed-on: https://chromium-review.googlesource.com/1033914
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#554599}(cherry picked from commit f0b02b2c35400a2cc1bd85094a5f02adf50ca92c)
Reviewed-on: https://chromium-review.googlesource.com/1035371
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#378}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/linux/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/mac-mac10.10/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/mac-mac10.12/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/mac/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/win/media/media-document-audio-repaint-expected.txt
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/WebKit/LayoutTests/platform/win7/media/media-document-audio-repaint-expected.png
[modify] https://crrev.com/a3999712776f6fd88caa4a882cf64c2a738a9fc9/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css
►

Sign in to add a comment