1377742 - about:rights shows XML parsing error

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Gert Van Waelvelde]

Attachment: screenshot.png

User Agent: Mozilla/5.0 (Windows NT 10.0; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170628075643

Steps to reproduce:

My device is a Samsung J7 running Android 6.0.1, Firefox 56.0a1 (2017-07-01).

1) Navigate to about:rights


Actual results:

XML Parsing Error: undefined entity 
Location: jar.jar:file:///data/app/org.mozilla.fennec_aurora-1/base.apk!/assets/omni.ja!/chrome/chrome/content/aboutRights.xhtml


Expected results:

about:rights page should have loaded without errors.

Comment 1

[Gert Van Waelvelde]

Update of error message (was impcomplete in original report):

XML Parsing Error: undefined entity 
Location: jar.jar:file:///data/app/org.mozilla.fennec_aurora-1/base.apk!/assets/omni.ja!/chrome/chrome/content/aboutRights.xhtml
Line Number 18, Column 10:

<title>&rights.pagetitle;</title>
-------^



Similar to bug #609069 ?

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Likely regressed by bug 1372005

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

This page is linked from about:firefox, a default bookmark.

Comment 4

[Dão Gottwald [:dao]]

Apparently Firefox for Android forked aboutRights.xhtml from toolkit (why?) but not aboutRights.dtd. Obviously this is a bad idea.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Recent regression, tracking. 
Dão or Sebastian, can you find someone to fix this issue? Thanks.

Comment 6

[Dão Gottwald [:dao]]

I could offer a band-aid fix, but it would be better if a Mobile engineer looked into untangling the half-assed fork of this page, see comment 4.

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Wesley, can you help find someone to investigate & fix? Thanks. 

In some ways this seems minor, but I think it's important to display the rights information correctly when we ship, so I'm marking this as a blocking issue for shipping fennec 56.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Dão Gottwald [::dao] from comment #6)
> I could offer a band-aid fix, but it would be better if a Mobile engineer
> looked into untangling the half-assed fork of this page, see comment 4.

A band-aid fix (or un-forking) might be a good idea. I guess everyone who knew why this was done isn't on the Fennec team anymore. Currently there are only two front-end engineers working on Fennec. Regressing it and then letting the Fennec team deal with it is problematic. :(

@Nevin, Jing-wei: Do you have time to look into this?

Comment 9

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

FW the NI to Wesly.

Comment 10

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1377742 - about:rights shows XML parsing error.

Review commit: https://reviewboard.mozilla.org/r/155952/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/155952/

Comment 11

[Nevin Chen(Not active on Bugzilla)]

Hi dao
I'm not familiar with this code.
Could you please give me some advise to fix it properly?
Thank you!

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8885100 [details]
Bug 1377742 - about:rights shows XML parsing error.

https://reviewboard.mozilla.org/r/155952/#review161162

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c96f0963b33
about:rights shows XML parsing error. r=dao

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9c96f0963b33

Comment 15

[Sorina Florean [:sflorean]]

Tried to verify this bug on latest Nightly build (2017-07-20) with devices: HTC 10 (Android 7.0), Honor 8 (Android 6.0) and the issue is still reproducible. When about:rights page is loaded the content is the same as in description.

Comment 16

[Wesly Huang (EPM)]

Nevin, would you help check? tks.

Comment 17

[Nevin Chen(Not active on Bugzilla)]

Hi Dao.
Could you please teach me how to fix this?
Thanks!

Comment 18

[Dão Gottwald [:dao]]

(In reply to Sorina Florean [:sorina] from comment #15)
> Tried to verify this bug on latest Nightly build (2017-07-20) with devices:
> HTC 10 (Android 7.0), Honor 8 (Android 6.0) and the issue is still
> reproducible. When about:rights page is loaded the content is the same as in
> description.

Not quite. The parser now can't find the &rights.title; entity. It used to be &rights.pagetitle;.

(In reply to Nevin Chen [:nechen] from comment #17)
> Hi Dao.
> Could you please teach me how to fix this?
> Thanks!

You're probably running into bug 515109. Why did you add back security.dtd in your patch?

Comment 19

[Jan Henning [:JanH]]

The whole situation is a bit confusing:

Testing with https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3791e8357839e9721bec9969a77fbf848ddee0ee&filter-searchStr=android%20api%2015%20opt%20(b)&selectedJob=113503718 (before Nevin's patch) and https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3791e8357839e9721bec9969a77fbf848ddee0ee&filter-searchStr=android%20api%2015%20opt%20(b)&selectedJob=113503718 (including Nevin's patch) it's broken both before and afterwards.

Testing Nightly builds with mozregression on the other hand, 2017-07-12 (http://ftp.mozilla.org/pub/mobile/nightly/2017/07/2017-07-12-10-03-08-mozilla-central-android-api-15/fennec-56.0a1.multi.android-arm.apk) is good and 2017-07-13 (the first build with this patch) is bad.

The strange thing however is that if I'm using the mozilla-central build with the same source revision as the official 2017-07-12 Nightly (https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=09a4282d1172ac255038e7ccacfd772140b219e2&filter-searchStr=android%20api%2015%20opt%20(b)&selectedJob=113585086), it's broken, too, so there's some funky interaction with our build configs as well?


[Tracking Requested - why for this release]:
And the 56 Beta hasn't made it to the Play Store yet, but testing with http://ftp.mozilla.org/pub/mobile/releases/56.0b1/android-api-15/multi/fennec-56.0b1.multi.android-arm.apk it's still broken...

Comment 21

[Nevin Chen(Not active on Bugzilla)]

NI myself so I remember to fix this. I added back security.dtd just want to copy toolkit/content/aboutRights.xhtml to mobile/android/chrome/content/aboutRights.xhtml.
I'll try to remove it and build again to see if it works.

Comment 22

[Nevin Chen(Not active on Bugzilla)]

I'm using a new bug 1390038 to fix this cause I don't want to backout previous code here

Comment 23

[Mike Taylor [:miketaylr]]

Based on Comment #22, let's close as a dupe.

Comment 24

[Mike Taylor [:miketaylr]]

(Actually, I'm not sure if I should have close this.... Comment #22 is referring to the extra dtd. Nevin, should this be reopened?)

Comment 25

[Nevin Chen(Not active on Bugzilla)]

I suggest we close this bug. Since this bug had a patch and the patch was landed.

Comment 26

[Mike Taylor [:miketaylr]]

OK, thanks.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 28

[Nevin Chen(Not active on Bugzilla)]

I've requested in bug 1390038 since the patch there fix the problem.

Comment 29

[u549602]

Verified as fixed on the latest Beta 56.0b5 build

Comment 30

[Sorina Florean [:sflorean]]

Verified as fixed on latest Nightly build. Device: Honor 8 (Android 7.0).