[Mac Harmony] Edit bookmarks sheet has a close button - ti should not |
||||||
Issue descriptionChrome Version: 65.0.3325.106 OS: macOS 10.13 What steps will reproduce the problem? (1) Navigate to a page (2) Click the bookmark star (3) Click Edit... What is the expected result? The sheet that appears should not have a close button. What happens instead? The sheet has a close button. I've lost track of Harmony's stance on close buttons is dialogs, but if you're going to use a macOS sheet to present a dialog, the sheet should be used correctly (i.e. per macOS UI guidelines). Sheets are document modal, meaning they have no close buttons/boxes and can only be dismissed by clicking a button at the bottom. I don't care so much what happens on other platforms as far as this dialog having a close button, but on the Mac there should be no close button (it looks like we don't know what we're doing).
,
Mar 22 2018
sdy's CL: https://chromium-review.googlesource.com/974784 but see also https://crbug.com/605657#c54 close [x] spec link: http://go/zlyko
,
Mar 22 2018
tapted@ - Based on my reading of issue 605657 #c50 and #c52, adjusting my change (https://crrev.com/c/974784) to: - Default to no close button for window-modal dialogs on all platforms. - Have the extension install dialog show a close button. …would give the correct behavior. Is that right?
,
Mar 22 2018
(Having one on the extensions dialog still feels wrong to me, but it sounds like trying to change it will involve a conversation with that team.)
,
Mar 22 2018
Unless we know of a dialog other than edit bookmark and extension install that's doing this, I think we should just fix those dialogs.
I'd do 2(3) CLs
- edit bookmark, all platforms
- extension install, just Mac + loop in relevant stakeholders with security concerns
- DCHECK in NativeWidgetMac::InitModalType like
if (modal_type == ui::MODAL_TYPE_WINDOW)
DCHECK(!GetWidget()->widget_delegate()->ShouldShowCloseButton());
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4cc54f06d87723977f0e9724432bff3b9b52135 commit c4cc54f06d87723977f0e9724432bff3b9b52135 Author: Sidney San Martín <sdy@chromium.org> Date: Fri Mar 23 19:29:41 2018 Remove the close button from the "Edit Bookmark" dialog. Bug: 823325 Change-Id: I8c8d9d866112a7fd7ed0ab1304bea4b8ad355ee1 Reviewed-on: https://chromium-review.googlesource.com/974784 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Sidney San Martín <sdy@chromium.org> Cr-Commit-Position: refs/heads/master@{#545555} [modify] https://crrev.com/c4cc54f06d87723977f0e9724432bff3b9b52135/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc [modify] https://crrev.com/c4cc54f06d87723977f0e9724432bff3b9b52135/chrome/browser/ui/views/bookmarks/bookmark_editor_view.h
,
Mar 23 2018
,
Mar 27 2018
Tested the issue on Mac OS 10.13.3 using chrome latest Canary M67-67.0.3381.0 by following steps mentioned in the original comment. Observed that Edit bookmark sheet displaying without close button as expected. Hence adding TE-Verified label. Please find the screen shot for reference. Thank you!
,
Mar 27 2018
Thanks! |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ellyjo...@chromium.org
, Mar 19 2018Status: Assigned (was: Untriaged)