1655040 - PIP is broken in Nightly 80.0a1 (2020-07-24) (64-bit) with Yellow Screen of Death

Status: RESOLVED WORKSFORME

(Toolkit :: Video/Audio Controls, defect, P1)

Description

[Alessio Placitelli [:Dexter]]

Attachment: pip_bug.png

XML Parsing Error undefined entity
Location: XXX
Line Number 20, Column 43:
<span class="pip-label">&pictureInPictureToggle.label;</span>

PiP is not usable in today's nightly. The above error is overlaid on any video.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

This is very strange. That entity is very much defined here:

https://searchfox.org/mozilla-central/rev/227f22acef5c4865503bde9f835452bf38332c8e/toolkit/locales/en-US/chrome/global/videocontrols.dtd#28

I can't wait until we're off DTDs for good.

Hey gandalf, I feel like this problem crops up periodically, where an entity that is defined still gives us YSOD. Did we ever determine what caused this? Is it startupcache related?

Comment 2

[Francesco Lodolo [:flod]]

I'm on 20200724093206 and it's working fine for me, although it's an Italian build, not en-US.
How do you reproduce? Do you just open a video where the control shows up?

Out of curiosity, can you attach here your raw about:support? After that, trying to clear up the start up cache from about:support might be a first try, although I'm not sure it's a cache problem.

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: support.txt

The steps to reproduce is just "open a youtube video" :)

Comment 4

[Francesco Lodolo [:flod]]

Please disable the language pack in about:addons, Languages pane :-(

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Gijs zoned in on the problem:

    {
      "name": "English (GB) Language Pack",
      "type": "locale",
      "version": "77.0buildid20200504222419",
      "isActive": true,
      "id": "[e-mail]"
    },

this language pack is several months old, for 77.

Comment 6

[Francesco Lodolo [:flod]]

Asked Alessio to also try manually searching for updates, and it didn't do anything (i.e. disable the add-on).

Apparently you can't disable add-ons anymore?

Comment 7

[Alessio Placitelli [:Dexter]]

(In reply to Francesco Lodolo [:flod] from comment #6)

Asked Alessio to also try manually searching for updates, and it didn't do anything (i.e. disable the add-on).

Apparently you can't disable add-ons anymore?

Uninstalling the language pack solves the problem, FWIW.

Comment 8

[Rob Wu [:robwu]]

Langpacks for Nightly aren't published on AMO. Any manually installed langpack will be stuck at that version.

How did you end up with the given langpack? Did you install it manually?

Comment 9

[Alessio Placitelli [:Dexter]]

(In reply to Rob Wu [:robwu] from comment #8)

Langpacks for Nightly aren't published on AMO. Any manually installed langpack will be stuck at that version.

How did you end up with the given langpack? Did you install it manually?

I honestly don't remember. Probably!

Comment 10

[Rob Wu [:robwu]]

The more general issue of incompatible langpacks after updates is tracked in bug 1566084.

I'll close this specific bug since there's nothing else to do here.

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Rob Wu [:robwu] from comment #8)

Langpacks for Nightly aren't published on AMO. Any manually installed langpack will be stuck at that version.

How did you end up with the given langpack? Did you install it manually?

Since AMO was reporting them as compatible, if the language switcher UI was enabled (it's disabled by default on Nightly and DevEdition), it was possible to install them directly from preferences for a couple of cycles (when 77 was on Beta and Release).

Comment 14

[:Gijs (he/him)]

(In reply to Rob Wu [:robwu] from comment #10)

The more general issue of incompatible langpacks after updates is tracked in bug 1566084.

But that bug is about the relatively difficult issue of making sure that we download a new langpack before updating, so that the update gets a working langpack immediately.

That's nice, but this bug is about an old langpack continuing to stay enabled and slowly wrecking UI when breaking changes happen. Can we at least avoid that, and disable the langpack when there's a version change that makes it incompatible? That seems easier than fixing bug 1566084 and would save us a lot of pain here - we'd fall back to a different language that actually works.

Comment 15

[Rob Wu [:robwu]]

Redirecting the question from comment 14 to Shane, since he is working on langpack stuff.

Comment 16

[Shane Caraveo (:mixedpuppy)]

I'm pretty sure this is a dup of bug 1646016 where AMO was updating the compat data to use maxVersion *. @flod pointed me to some telemetry which seems to show only a tiny exposure[1] on that problem. Given that, my preference is merely to address bug 1646016 and prevent compat data from being written for langpacks in the future. The only other solution I can think of is to re-read the langpack manifest on startup rather than using cached data, which could be a startup perf hit.

What do you think Gijs, is this a more serious problem than it appears?

[1] https://sql.telemetry.mozilla.org/queries/73274

Comment 17

[:Gijs (he/him)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #16)

I'm pretty sure this is a dup of bug 1646016 where AMO was updating the compat data to use maxVersion *. @flod pointed me to some telemetry which seems to show only a tiny exposure[1] on that problem.

Given how many dupes we're seeing of this problem, I am skeptical of the "tiny exposure" metric.

Given that, my preference is merely to address bug 1646016 and prevent compat data from being written for langpacks in the future.

I don't know what this means and what it would do to address this bug. Can you elaborate?

The only other solution I can think of is to re-read the langpack manifest on startup rather than using cached data, which could be a startup perf hit.

I don't like startup perf hits, but I like this problem even less. Who would be incurring the startup perf hit - just people with this langpack? And at every startup or only after app updates or something else? And are we re-reading from disk or from network or from something else? (unsure what "langpack manifest" means in this context)
Could we restrict it to langpacks that match the application language? They seem to somehow be causing more of an issue.

What do you think Gijs, is this a more serious problem than it appears?

I think we're still unsure of frequency (because the number of bugreports and the numbers in telemetry contradict each other), and it's a pretty severe problem, so I think we should do something.

Comment 18

[Francesco Lodolo [:flod]]

(In reply to :Gijs (he/him) from comment #17)

(In reply to Shane Caraveo (:mixedpuppy) from comment #16)

I'm pretty sure this is a dup of bug 1646016 where AMO was updating the compat data to use maxVersion *. @flod pointed me to some telemetry which seems to show only a tiny exposure[1] on that problem.

Given how many dupes we're seeing of this problem, I am skeptical of the "tiny exposure" metric.

We're going to see more coming from Linux (with telemetry disabled by default). The only hope is that most users install language packs through the system or package manager, they don't switch via preferences.

Since there are only 2 dupes in this bug: the obsolete langpack also causes jank, memory/cpu issues, and we've seen a bunch of those (bug 1642415).

Comment 19

[Shane Caraveo (:mixedpuppy)]

To my knowledge, between the AMO fix and bug 1646016 the issue of old langpacks continuing to work should be addressed.