New style notifications: long domain names move action buttons off view |
|||
Issue descriptionChrome Version: 71.0.3578.98 OS: Linux What steps will reproduce the problem? (1) Show a new style notification for a site with long domain name (2) Observe notification view What is the expected result? The long domain name should be properly shortened, so that the settings and close buttons still fit on screen. What happens instead? No buttons are visible and the domain name shortening is inconsistent. When the site requests permissions to show notifications, the domain name shortening is different (the end of the domain name is visible) to the one used in the notification title bar (the start of the domain name is visible). Not sure which of the two is preferred from a security perspective.
,
Dec 19
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b39f64487df3ba8178a753859d7deabfdb57b4d commit 6b39f64487df3ba8178a753859d7deabfdb57b4d Author: Richard Knoll <knollr@chromium.org> Date: Thu Dec 20 21:09:00 2018 cleanup: reuse UpdateControlButtonsVisibility among notification views Move UpdateControlButtonsVisibility up to |MessageView| and removing unused code. This is a preparation to fix the |NotificationHeaderView|. Note: |ArcNotificationView| is now the only class overriding this. Bug: 915224 Change-Id: Ide72668c7bb88c63148c43e756061bbdb9ff13eb Reviewed-on: https://chromium-review.googlesource.com/c/1386624 Commit-Queue: Richard Knoll <knollr@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/heads/master@{#618322} [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ash/media/media_notification_view.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ash/media/media_notification_view.h [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ash/system/message_center/arc/arc_notification_content_view.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/message_view.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/message_view.h [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_control_buttons_view.h [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_view.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_view.h [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_view_md.cc [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_view_md.h [modify] https://crrev.com/6b39f64487df3ba8178a753859d7deabfdb57b4d/ui/message_center/views/notification_view_md_unittest.cc
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8defb9b5b1cc44033c22761a21a5fec392f430c9 commit 8defb9b5b1cc44033c22761a21a5fec392f430c9 Author: Richard Knoll <knollr@chromium.org> Date: Thu Dec 20 21:23:55 2018 fix: properly layout the notification header and elide urls We use two nested |BoxLayout|s to layout the notification header. This CL uses the shiny new |FlexLayout| which is already in use by the |ToolbarView| to prevent very long URLs from moving the control buttons off the side. Also set the correct text elide behaviour to trim URLs from the start to see the TLDs. Bug: 915224 Change-Id: I662eb4e7f71356e8d441229c2e01b7b2da0caa95 Reviewed-on: https://chromium-review.googlesource.com/c/1384372 Commit-Queue: Richard Knoll <knollr@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#618327} [modify] https://crrev.com/8defb9b5b1cc44033c22761a21a5fec392f430c9/ui/message_center/views/notification_header_view.cc [modify] https://crrev.com/8defb9b5b1cc44033c22761a21a5fec392f430c9/ui/message_center/views/notification_header_view.h [modify] https://crrev.com/8defb9b5b1cc44033c22761a21a5fec392f430c9/ui/message_center/views/notification_view_md.cc
,
Dec 20
|
|||
►
Sign in to add a comment |
|||
Comment 1 by peter@chromium.org
, Dec 14