Remove extra spacing from clear browsing data bubble for PT/Tor windows

[kjozwiak]

Description

After removing the images from the clear browsing data bubble for PT/Tor windows via brave/brave-core#12643, you'll notice that there's some extra space that can be removed to clean things up.

Steps to Reproduce

1. open a Tor or PB window
2. select the Hamburger Menu -> More Tools -> Clear browsing data
3. you'll notice the extra spacing that appears via the bubble

Actual result:

Tor Window Example	PB Window Example

Expected result:

We should remove the extra spacing after removing the image from the modal via brave/brave-core#12643.

Reproduces how often:

100% reproducible using the STR/Cases outlined above.

Brave version (brave://version info)

Brave | 1.38.57 Chromium: 99.0.4844.74 (Official Build) nightly (64-bit)
-- | --
Revision | fee9a47e86e981802390cb0d41c5ed7ea93c4f6f-refs/branch-heads/4844@{#1060}
OS | Windows 11 Version 21H2 (Build 22000.556)

Version/Channel Information:

• Can you reproduce this issue with the current release? Yes
• Can you reproduce this issue with the beta channel? Yes
• Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

• Does the issue resolve itself when disabling Brave Shields? N/A
• Does the issue resolve itself when disabling Brave Rewards? N/A
• Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

[AlexNguyen1612]

Hi @kjozwiak, I'm totally new to open source development and currently interested in picking up some #good-first-issue, just wondering if you are okay for me to pick this up?

Thank you

[aghio-2020]

Hey @kjozwiak! I'm new to open source and I was looking for a good first issue to work on. It's okay for me to pick this one? Thanks.

[bsclifton]

Hi folks - yes; please do investigate and check out 😄 Let us know if you have any questions

@kjozwiak can you share more about the spacing (where is the spacing?) Around the borders? or between the two sentences?

[AlexNguyen1612]

Hi @bsclifton, thank you for your reply, does Brave have an active Slack channel (or something similar) that I can join and reach out to you guys in real time?

[kjozwiak]

Hi folks - yes; please do investigate and check out 😄 Let us know if you have any questions

@kjozwiak can you share more about the spacing (where is the spacing?) Around the borders? or between the two sentences?

Once we removed the Chrome images via brave/brave-core#12643, there's a small space left at the top where the image used to be. We could probably remove some of that space at the top. It looks unsymmetrical with no space at the bottom but a bunch of space at the top. I think it would look better if we move the text a bit higher in that model now that the images have been removed. CCing @rebron just in case but I remember we talked about removing this space during one of the Tuesday's b-b meetings 👍

[AlexNguyen1612]

Hi @kjozwiak and @bsclifton, sorry for the delay response, it took me quite a while to get Brave set up properly and I also spent some time to explore the Brave code base! I've removed the header art view so that the text moves up a bit higher, can you please confirm if this is the desired result or we need to bring it even closer to the top with minimal space with the border?

I've attached some screenshots below, including the comparison of two versions for your reference
Thank you

Final version

[kjozwiak]

Definitely looks a lot better compared to before 👍CCing @rebron @bradleyrichter

[AlexNguyen1612]

Hi @kjozwiak, thank you for confirming 😄
Please review my PR and let me know if there's any problem

[bsclifton]

I think this behavior is expected and the patch you submitted @AlexNguyen1612 might not be the correct solution (we want to minimize patches; see our patching guide here: https://github.com/brave/brave-browser/wiki/Patching-Chromium

Maybe we can add a chromium_src override for IncognitoClearBrowsingDataDialog::SetDialogForHistoryDisclaimerBubbleType and try to prevent the X from being added into the view too (which might be reserving the entire horizontal space; looks like a title area)
cc: @mkarolin for patching insights

cc: @fallaciousreasoning @sangwoo108 @petemill for Chromium views

[fallaciousreasoning]

You might also be able to remove the close button with SetShowCloseButton in chrome/browser/ui/views/incognito_clear_browsing_data_dialog.cc#77 (via a patch like @bsclifton mentioned)

[sangwoo108]

Hi @AlexNguyen1612 ! You're almost there! I made a rough patch to help you override the dialog https://github.com/brave/brave-core/compare/sko/dialog-stub?expand=1 . You can do what you want in the ctor. (SetDialogForHistoryDisclaimerBubbleType occurred twice in .cc file. so it was too tricky to override it and I detoured) . This may not be the best way but I hope this can help you.

Like @fallaciousreasoning said, you can use SetShowCloseButton()
And to find the Tracking view, you can use views::View::children() , views::View::GetClassName() and ThemeTrackingNonAccessibleImageView::kViewClassName. Class name related stuffs are defined by METADATA_HEADER macro. In case you wanna try adjusting margins, you can use set_margins(). Go for it!

[AlexNguyen1612]

Hi @bsclifton, @fallaciousreasoning and @sangwoo108
Thank you all for the suggestions, I'm looking into it @sangwoo108 and will play around a bit with the overriden ctor 👍
I tried to override SetDialogForHistoryDisclaimerBubbleType a few days ago but couldn't figure it out so was kinda left this hanging. Hopefully I can do something with this new patch!

[ArshErgon]

is it still open?

[fallaciousreasoning]

It looks like @AlexNguyen1612 pushed a commit a couple of days ago in his branch, so I'd say the issue is still open, but being worked on 😄

[bsclifton]

Added missing milestone 👍

[stephendonner]

Verified PASSED using

Brave	1.44.73 Chromium: 105.0.5195.68 (Official Build) beta (x86_64)
Revision	ad13e82529051bac6a0e65f455e6d7a1e5fd7938-refs/branch-heads/5195@{#903}
OS	macOS Version 13.0 (Build 22A5331f)

Steps:

1. installed 1.44.73
2. launched Brave
3. launched a Private window via "hamburger menu" -> New Private Window
4. launched a Private window with Tor via "hamburger menu" -> New Private Window with Tor
5. launched a Guest window via "hamburger menu" -> Open Guest Window
6. in each, selected "hamburger menu" -> More Tools -> Clear Browsing Data...
7. examined and confirmed fixed vertical spacing in the Cancel Close Windows dialog

Private window	Private window with Tor	Guest window (unavailable)