Fix tor-browser tbb-tests

With current rebased tor-browser ESR68 branch I can only run tbb-tests (with run-tbb-tests script) when pref("network.file.path_blacklist", "/net") is removed and pref("extensions.torbutton.use_nontor_proxy", true); is set, apart from disabling tor-launcher. The second pref disables the domain isolator, which makes sense since it expects SOCKS5 proxies, but mochitests override that. For the other pref, not sure why network.file.path_blacklist needs to be unset (at least for Linux).

We could put these prefs in testing/marionette/prefs/marionette.js so that tests can be run (unless there is a simpler way to get the tests tor run that I'm missing).

Trac:
Parent Ticket: legacy/trac#33654 (moved)
Child Ticket(s): legacy/trac#17809 (moved), legacy/trac#19670 (moved)

added ReleaseTrainMigration in Legacy / Trac TorBrowserTeam202006R in Legacy / Trac actualpoints::3 in Legacy / Trac component::applications/tor browser in Legacy / Trac owner::acat in Legacy / Trac parent::33654 in Legacy / Trac priority::medium in Legacy / Trac reviewer::gk in Legacy / Trac reviewer::sysrqb in Legacy / Trac severity::normal in Legacy / Trac sponsor::58-must in Legacy / Trac status::needs-review in Legacy / Trac type::defect in Legacy / Trac labels

Adding Sponsor 44 to ESR68 tickets

Trac:
Sponsor: N/A to Sponsor44-can

Trac:
Sponsor: Sponsor44-can to Sponsor58

Trac:
Summary: Fix tor-browser tbb-tests for ESR68 to Fix tor-browser tbb-tests

Removing ff68-esr keyword, I think it's more realistic to aim for ReleaseTrainMigration.

Trac:
Keywords: ff68-esr deleted, ReleaseTrainMigration added

This took longer than expected.

Here is a branch with 6 fixups for tests: https://github.com/acatarineu/tor-browser/commits/30832.

• f5202200d1ecdc56f0437f83210cb292ae40559e: the old

Replying to acat:

• c73bfe7f3b21421c7349267f2abac9f90f503b73: we need layout.css.line-height.normal-as-resolved-value.enabled = false, otherwise line-height property is read as normal and we cannot get the number in pixels

You should update that to fall back to clientRect (which I do on TZP): you'll need to anyway once you are based on ESR78 (or rapid release)

Replying to Thorin:

Replying to acat:

• c73bfe7f3b21421c7349267f2abac9f90f503b73: we need layout.css.line-height.normal-as-resolved-value.enabled = false, otherwise line-height property is read as normal and we cannot get the number in pixels

You should update that to fall back to clientRect (which I do on TZP): you'll need to anyway once you are based on ESR78 (or rapid release)

Thanks for the suggestion. I spent some time playing with getBoundingClientRect() to find out the line-height, but I saw some inconsistent behaviour. One of the testscases in test_tor_bug23104.html has arabic characters, which sets its font to Noto Naskh Arabic. That one, despite reporting a line-height of 20.4px, when measured via getBoundingClientRect() it turns out to be 29px (using a div to measure, instead of a span, the height of which is not affected by line-height). I assume that there must be some metric of the arabic font which is affecting the actual height of the element, which is different than the reported in line-height.

I'm not sure if the issue that legacy/trac#23104 (moved) tried to fix is still there, and can be measured via getBoundingClientRect or similar. In any case, the fact that the actual height does not match the line-height with some fonts (e.g. Noto Naskh Arabic) seems to be a different issue. So perhaps we can just leave the test as it is for now (using layout.css.line-height.normal-as-resolved-value.enabled = false) and investigate it later.

Still, I was thinking that the issues in legacy/trac#23104 (moved) and legacy/trac#29563 (moved) only allow distinguishing platform, but not subplatform, right? If that's the case, perhaps we could consider just dropping the patch for legacy/trac#23104 (moved), as leaking the os (via fonts?) is not something that we can really avoid.

Trac:
Actualpoints: 2 to 3

Replying to acat:

using a div to measure, instead of a span

Yeah. ... dcf's unicode glyphs test (which is also used as a base on TZP) - https://www.bamsoftware.com/talks/fc15-fontfp/fontfp.html#demo

From my notes // Read the span width, but the div height. Firefox always reports the same value for the span's offsetHeight, even if the div around it is changing size

---

I don't see a problem with "leaking" the major OS (Win, Mac, Android, Linux: we'll never stop that), its the entropy across Linux (and Android for now, see below) that worries me. And we should always make it as hard as possible for the bastards. The patch is still protecting most/all users from alternative measuring - e.g. domrect.

FWIW: I also see occasional weird measurements, e.g. with textMetrics, with various codepoints and/or fonts: still trying to pinpoint exactly what it is: but I think it might mainly be to do with right-to-left.

I'm not sure I see the issue with Noto Naskh Arabic: if all users are the same, then it's OK. Sure, it's not 19.2, but it might just be an odd outlier (for everyone?). If someone is spoofing as English - their default font is Times New Roman. And if they're not, then they are already different: navigator languages etc - e.g. TB-fa. The question is, is everyone the same?

legacy/trac#23104 (moved) also lacks parity effectiveness in Android (and least on my phone): it's doing something (toggle RFP) but it sure isn't 19.2 either. I sent sysrqb an email about it months ago (last December) - it's something to with the 19.2 is only returned at certain zoom levels: and on my Android phone, the devicePixelRatio is 2.609... and it screws up - so it's really a case of this being a symptom of legacy/trac#29563 (moved)

---

At the end of the day, even once ESR78 is used (or you flip the pref early: there's nothing to say you can't do that now), I would still keep the patch: because domrect can still measure it

Keep the test, close this, and re-purpose legacy/trac#29563 (moved) to deal with anomalies (getting it always 19.2 across the board would neutralize domrect in lineheight)?

Trac:
Parent: N/A to legacy/trac#33654 (moved)

Trac:
Sponsor: Sponsor58 to Sponsor58-must

Trac:
Status: new to assigned
Owner: tbb-team to acat

Trac:
Status: assigned to needs_review
Keywords: ReleaseTrainMigration deleted, ReleaseTrainMigration TorBrowserTeam202005R added
Cc: N/A to tbb-team

Trac:
Reviewer: N/A to gk

Moving review tickets.

Trac:
Keywords: ReleaseTrainMigration TorBrowserTeam202005R deleted, TorBrowserTeam202006R, ReleaseTrainMigration added

Trac:
Reviewer: gk to gk,sysrqb

added 24h of time spent

mentioned in issue legacy/trac#33791 (moved)

mentioned in issue legacy/trac#33654 (moved)

moved from legacy/trac#30832 (moved)

added Needs Review label

added 1 deleted label

added Bug label and removed 1 deleted label

mentioned in issue #33654 (closed)

assigned to @gk and unassigned @acat

Assigned this to @gk as he is the reviewer.

added Release Train Migration label and removed 1 deleted label

removed 1 deleted label

removed milestone

mentioned in merge request tor-browser-bundle-testsuite!1 (closed)

assigned to @acat and unassigned @gk

I will rebase and make a MR.

added Doing label and removed Needs Review label

mentioned in merge request !74 (closed)

added Needs Review label and removed Doing label

changed milestone to %Tor Browser: 10.5

I wonder what the non-ESR story is. @acat: do we need an own patch, adapting the tests for that? One is failing there:

tbb-tests/browser_tor_omnibox.js
  FAIL Secondary search engine is youtube - Got "ddg-onion", expected "youtube"
Stack trace:
chrome://mochikit/content/browser-test.js:test_is:1332
chrome://mochitests/content/browser/tbb-tests/browser_tor_omnibox.js:null:12

Actually that's a regression, apparently this test is a bit more comprehensive than the one in the testsuite. I created #40288 (closed).

added Needs Information label and removed Needs Review label

removed Needs Information label

added Needs Information label

I cherry-picked the patches onto tor-browser-85.0b1-10.5-1, too, including my fixup commit (9103b99a).

@acat: There is a discussion in #30832 (comment 2664115) f. If there are follow-up items to work on could you file additional tickets? Otherwise please close this bug.

Filed #40290 (moved).

mentioned in issue #40290 (moved)

closed

mentioned in issue tor-browser-bundle-testsuite#40059