Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

All <span> tags in hover have a 4px bottom margin which causes nested spans to have stacking bottom margins #232226

Open
navtej-ac opened this issue Oct 25, 2024 · 4 comments · May be fixed by #232228
Open

All <span> tags in hover have a 4px bottom margin which causes nested spans to have stacking bottom margins #232226

navtej-ac opened this issue Oct 25, 2024 · 4 comments · May be fixed by #232228
Assignees
@aiday-mar
Labels
bug Issue identified by VS Code Team member as probable bug editor-hover Editor mouse hover

Comments

@navtej-ac
Copy link

navtej-ac commented Oct 25, 2024 •
edited
Loading

Type: Bug

When an extension provides a hover using the HoverProvider.provideHover API, and the hover contains Markdown with HTML in it, every span gets a 4px bottom margin. This was added in #107442 due to microsoft/vscode-pull-request-github#1937

This means that nested spans will get stacking margins, and vertically stacked spans will have gaps between them which may be undesirable depending on how the author of the extension wants to style the hover.

Because of the (understandably) restrictive CSS allowed in markdownRenderer.ts, we cannot remove this bottom margin.

I would consider the stacking margins with nested spans to be a bug, because nested spans may be required to do certain types of styling of text and it does not seem correct for that to cause the margin to expand with each level of nesting.

Demonstration of problem:
Image

To reproduce this:

  1. build and install the extension at https://github.com/navtej-ac/vscode-hover-sample-extension ... It is based on the Hello World Minimal Sample from the vscode-extension-samples repo.
  2. With the extension installed, activate it by running the "Hello World" command
  3. Then hover on any line the text editor. On every other line we show the hover with background colors and demonstrate both the gap between spans and the background color and the stacking of margins. On the remaining lines we try to show a hover with margin:0 added to the spans' styling, which causes the HTML sanitizer to strip the styles

Expected: spans with background colors can nest without the outer span increasing in height and spans with backgrounds do not have a vertical gap between them.

Actual: nested spans all have 4px margins which accumulate and vertically stacked spans have a gap between them.

Proposed Fix: Add margin:0 to the list of allowed styles in markdownRenderer.ts.

VS Code version: Code 1.94.2 (Universal) (384ff73, 2024-10-09T16:08:44.566Z)
OS version: Darwin arm64 23.6.0
Modes:

System Info
Item Value
CPUs Apple M3 (8 x 2400)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_graphite: disabled_off
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
webgpu: enabled
webnn: disabled_off
Load (avg) 11, 9, 9
Memory (System) 16.00GB (0.40GB free)
Process Argv --disable-extensions --crash-reporter-id 05b8a457-1897-4b62-8507-c739f0441e0b
Screen Reader no
VM 0%
Extensions disabled
A/B Experiments 
vsliv368:30146709
vspor879:30202332
vspor708:30202333
vspor363:30204092
vscod805cf:30301675
binariesv615:30325510
vsaa593:30376534
py29gd2263:31024239
vscaac:30438847
c4g48928:30535728
azure-dev_surveyone:30548225
vscrpc:30673769
2i9eh265:30646982
962ge761:30959799
pythongtdpath:30769146
pythonnoceb:30805159
asynctok:30898717
pythonmypyd1:30879173
h48ei257:31000450
pythontbext0:30879054
cppperfnew:31000557
dsvsc020:30976470
pythonait:31006305
dsvsc021:30996838
f3je6385:31013174
dvdeprecation:31068756
dwnewjupyter:31046869
impr_priority:31102340
nativerepl1:31139838
refactort:31108082
pythonrstrctxt:31112756
wkspc-onlycs-t:31132770
wkspc-ranged-t:31151552
cf971741:31144450
defaultse:31146405
iacca2:31156134
notype1:31157159
5fd0e150:31155592
dwcopilot:31164048
iconenabled:31158251
navtej-ac added a commit to navtej-ac/vscode that referenced this issue Oct 25, 2024
@navtej-ac
issue microsoft#232226: Allow margin:0 in markdownRenderer so that we…
fe05a72 
… can override the 4px bottom margin that is added to every span in the Hover
@navtej-ac navtej-ac linked a pull request Oct 25, 2024 that will close this issue
Open
@Codimow
Copy link

Codimow commented Oct 25, 2024 •
edited
Loading

Hover Margin Stacking Issue in VS Code

Issue Description

The current hover implementation in VS Code adds a 4px bottom margin to all <span> tags in hover content (introduced in #107442). While this was intended to improve GitHub PR hover spacing, it's causing unintended margin stacking in nested structures.

Problematic Behavior

  • All hover <span> elements receive a 4px bottom margin
  • Nested spans accumulate margins (4px * nesting level)
  • Vertically stacked spans show undesired gaps

Root Cause

The margin is being applied universally through CSS, but the markdown renderer's sanitizer prevents extensions from overriding these margins. This becomes particularly problematic with:

  • Nested span structures
  • Spans with background colors
  • Complex hover layouts

Proposed Fix

// markdownRenderer.ts
private static readonly ALLOWED_MARKDOWN_CSS = new Set([
    // ... existing allowed styles ...
    'margin',
    'margin-bottom'
]);

And update hover styles:

.hover-contents > span {  /* Direct children only */
    margin-bottom: 4px;
}
.hover-contents span span {  /* Nested spans */
    margin-bottom: 0;
}

This solution:

  • Preserves spacing for top-level elements
  • Prevents margin stacking in nested structures
  • Maintains backwards compatibility

Testing

Verified with the reproduction case in the hover sample extension. No regressions observed in standard hover behaviors.

/cc @microsoft/vscode-team
/label bug
/label area-hover

@aiday-mar aiday-mar added the editor-hover Editor mouse hover label Oct 25, 2024
@aiday-mar aiday-mar added the bug Issue identified by VS Code Team member as probable bug label Dec 9, 2024
@navtej-ac
Copy link
Author

Hi @aiday-mar, is there anything else you need from me to get my fix for this issue merged?
Please let me know if I can help in any way. Thanks!

@aiday-mar
Copy link
Contributor

Hi @navtej-ac thanks for filing this issue. The change you made touches upon an area which another colleague of mine is responsible for. I assigned him as a reviewer.

@navtej-ac
Copy link
Author

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aiday-mar aiday-mar

Labels
bug Issue identified by VS Code Team member as probable bug editor-hover Editor mouse hover
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: #232226 - Allow margin:0 in markdownRenderer navtej-ac/vscode
3 participants
@aiday-mar @navtej-ac @Codimow