Closed Bug 1592243 Opened 4 years ago Closed 4 years ago

Netmonitor Blocking - block list truncates bottom part of diacritics on Windows 10

Categories

(DevTools :: Netmonitor, defect, P3)

defect

Tracking

(firefox-esr68 unaffected, firefox70 unaffected, firefox71 wontfix, firefox72 wontfix, firefox74 wontfix, firefox75 wontfix, firefox76 verified)

RESOLVED FIXED
Firefox 76
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- verified

People

(Reporter: cfogel, Assigned: lelouch.cpp)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, regression)

Attachments

(4 files)

Affected versions

  • 71.0b5, 72.0a1 (2019-10-28)ș

Affected platforms

  • Windows 10;

Steps to reproduce
devtools.netmonitor.features.requestBlocking set on true;

  1. Launch Firefox, open DevTools - Netmonitor;
  2. Enable the (request)Blocking panel;
  3. In the Block resource when URL contains input field add the following string:
    Ç ç Ñ ñ Ã ã Č č Š š Ä ä Ö ö È è Ò ò É é Á á Â â Î î Ă ă Ș ș Ț ț

Expected result

  • each letter is properly displayed;

Actual result

  • letters that have the added glyph_symbol under the letter appear truncated
    (Ç ç Ş şȚ ț )

Regression range

  • not a regression;

Additional notes

  • attached screenshot with the issue;
Has STR: --- → yes
Summary: [win]Netmonitor Blocking - block list truncates bottom part of diacritics → Netmonitor Blocking - block list truncates bottom part of diacritics on Windows 10

Good catch, thanks for the report Cristian!

Honza

Priority: -- → P3

Can I take a shot at this issue if it isn't already taken?
Thank you

(In reply to Kriyszig from comment #3)

Can I take a shot at this issue if it isn't already taken?

Assigned to you, thanks for the help.

Honza

Assignee: nobody → lelouch.cpp
Status: NEW → ASSIGNED

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

I would require some comment on which approach to take. The root cause of the bug seems to be the font face being used to render the component and I tried a couple of font face with a few more CSS tweaks.
On using the same font as this forum, the result can be seen in the previous attachment. The above attachment shows the use of san-seriff which In my humble opinion renders the bottom of the glyph text better (the extensions are comparatively longer) but the font isn't as fluid as the previous example. Which one is a better choice?

I would love a second opinion before I submit a patch.

Yours sincerely,
Kriyszig

Florens, what is the right approach here?
Do we have to change the font face?
Would some CSS tricks be enough?

Honza

Flags: needinfo?(florens)

I don't think the font is the culprit here. We can have different results with different fonts because they won't draw their baseline and diacritics exactly the same, but the core reason is that we display the labels in a 16px-tall span styled like this:

.request-blocking-label {
  flex: 1 1 auto;
  display: block;
  white-space: nowrap;
  overflow: hidden;
  text-overflow: ellipsis;
  font-family: var(--monospace-font-family);
  line-height: 16px;
}

The 16px height is apparently not enough to draw some diacritics, and the overflow: hidden (needed for the ellipsis) cuts the overflowing part off.

One possible workaround is to add some vertical padding the the span:

.request-blocking-label {
  ...
  padding-block: 2px;
}

But, to keep the row height at 20px tall, we would need to remove the 2px vertical padding from the parent element, which is the <label> in:

<label class="devtools-checkbox-label">
  <input type="checkbox" class="devtools-checkbox">
  <span class="request-blocking-label request-blocking-editable-label" title="Š">Š</span>
</label>

But that style is shared with other parts of DevTools, so just changing it could cause regressions. I'm not fond of using the devtools-checkbox styles here anyway, so maybe we should change those classes to something more specific to the Request Blocking component, for example:

<label class="request-blocking-checkbox-label">
  <input type="checkbox" class="request-blocking-checkbox">
  <span class="request-blocking-label request-blocking-editable-label" title="Š">Š</span>
</label>

... and copy the relevant styles from .devtools-checkbox-label with the new class names in the Request Blocking stylesheet, modifying them as we see fit.

Flags: needinfo?(florens)

Thank you for these valuable insights. I'll make the changes accordingly based on them.
Thank you very much for your time.

Yours sincerely,
Kriyszig

The bottom part of the diactrics are truncated as the line size
of 16px coupled with the tight padding and overflow hidden isn't
enough to render them in the list.

Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f58bd341b2f
Fix diacritics truncation in network blocking panel r=Honza
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76

The patch landed in nightly and beta is affected.
:lelouch.cpp, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(lelouch.cpp)

Comment on attachment 9133358 [details]
Bug 1592243 - Fix diacritics truncation in network blocking panel r=Honza,fvsch

Beta/Release Uplift Approval Request

  • User impact if declined: Less readable text with diacritics (only for web developers)
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk, only web developers, one line CSS change.
  • String changes made/needed:
Attachment #9133358 - Flags: approval-mozilla-beta?

Comment on attachment 9133358 [details]
Bug 1592243 - Fix diacritics truncation in network blocking panel r=Honza,fvsch

This is not a new issue I think it can ride the trains.

Flags: needinfo?(lelouch.cpp)
Attachment #9133358 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Verified with 76.0a1 (2020-03-22).

You need to log in before you can comment on or make changes to this bug.