about:rights shows XML parsing error

VERIFIED FIXED in Firefox 56

Status

()

Firefox for Android
General
VERIFIED FIXED
a year ago
9 months ago

People

(Reporter: Gert Van Waelvelde, Assigned: Nevin Chen(Not active on Bugzilla))

Tracking

({regression})

56 Branch
Firefox 57
All
Android
regression
Points:
---

Firefox Tracking Flags

(fennec+, firefox54 unaffected, firefox55 unaffected, firefox56blocking verified, firefox57+ verified)

Details

(Whiteboard: [FNC][SPT57.2][INT][SP=X, 3])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8882859 [details]
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.
(Reporter)

Comment 1

a year ago
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 ?
(Reporter)

Updated

a year ago
Component: General → XML
Product: Firefox for Android → Core
Likely regressed by bug 1372005
Blocks: 1372005
tracking-fennec: --- → ?
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
status-firefox56: --- → affected
Keywords: regression
tracking-firefox56: --- → ?
Status: UNCONFIRMED → NEW
Component: XML → General
Ever confirmed: true
Product: Core → Firefox for Android
This page is linked from about:firefox, a default bookmark.
Apparently Firefox for Android forked aboutRights.xhtml from toolkit (why?) but not aboutRights.dtd. Obviously this is a bad idea.
Recent regression, tracking. 
Dão or Sebastian, can you find someone to fix this issue? Thanks.
tracking-firefox56: ? → +
Flags: needinfo?(s.kaspari)
Flags: needinfo?(dao+bmo)
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.
Flags: needinfo?(dao+bmo)
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.
tracking-firefox56: + → blocking
Flags: needinfo?(whuang)
Flags: needinfo?(jcheng)
(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?
Flags: needinfo?(topwu.tw)
Flags: needinfo?(s.kaspari)
Flags: needinfo?(cnevinchen)
FW the NI to Wesly.
Flags: needinfo?(whuang) → needinfo?(wehuang)
Comment hidden (mozreview-request)
(Assignee)

Comment 11

11 months ago
Hi dao
I'm not familiar with this code.
Could you please give me some advise to fix it properly?
Thank you!
Flags: needinfo?(cnevinchen)
(Assignee)

Updated

11 months ago
Assignee: nobody → cnevinchen

Comment 12

11 months ago
mozreview-review
Comment on attachment 8885100 [details]
Bug 1377742 - about:rights shows XML parsing error.

https://reviewboard.mozilla.org/r/155952/#review161162
Attachment #8885100 - Flags: review?(dao+bmo) → review+

Comment 13

11 months ago
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c96f0963b33
about:rights shows XML parsing error. r=dao
(Assignee)

Updated

11 months ago
tracking-fennec: ? → +

Updated

11 months ago
Flags: needinfo?(topwu.tw)

Comment 14

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9c96f0963b33
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
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

11 months ago
Nevin, would you help check? tks.
Flags: needinfo?(wehuang) → needinfo?(cnevinchen)
(Assignee)

Comment 17

11 months ago
Hi Dao.
Could you please teach me how to fix this?
Thanks!
Flags: needinfo?(cnevinchen) → needinfo?(dao+bmo)

Comment 18

11 months ago
(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?
Flags: needinfo?(dao+bmo)
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...
Status: RESOLVED → REOPENED
status-firefox56: fixed → affected
status-firefox57: --- → affected
tracking-firefox57: --- → ?
OS: Unspecified → Android
Hardware: Unspecified → All
Resolution: FIXED → ---
Duplicate of this bug: 1389659
(Assignee)

Comment 21

10 months ago
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.
Flags: needinfo?(cnevinchen)
(Assignee)

Comment 22

10 months ago
I'm using a new bug 1390038 to fix this cause I don't want to backout previous code here
Flags: needinfo?(cnevinchen)
Based on Comment #22, let's close as a dupe.
Status: REOPENED → RESOLVED
Last Resolved: 11 months ago10 months ago
Flags: needinfo?(jcheng)
Resolution: --- → DUPLICATE
Duplicate of bug: 1390038
(Actually, I'm not sure if I should have close this.... Comment #22 is referring to the extra dtd. Nevin, should this be reopened?)
Status: RESOLVED → REOPENED
Flags: needinfo?(cnevinchen)
Resolution: DUPLICATE → ---
(Assignee)

Comment 25

10 months ago
I suggest we close this bug. Since this bug had a patch and the patch was landed.
Flags: needinfo?(cnevinchen)
OK, thanks.
Status: REOPENED → RESOLVED
Last Resolved: 10 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: Firefox 56 → Firefox 57
Please request Beta approval on this when you get a chance.
status-firefox57: affected → fixed
Flags: needinfo?(cnevinchen)

Updated

10 months ago
tracking-firefox57: ? → +
(Assignee)

Comment 28

10 months ago
I've requested in bug 1390038 since the patch there fix the problem.
Flags: needinfo?(cnevinchen)

Comment 29

10 months ago
Verified as fixed on the latest Beta 56.0b5 build
status-firefox56: affected → verified

Updated

10 months ago
Whiteboard: [FNC][SPT57.2][INT]
(Assignee)

Updated

10 months ago
Whiteboard: [FNC][SPT57.2][INT] → [FNC][SPT57.2][INT][SP=X, 3]
Verified as fixed on latest Nightly build. Device: Honor 8 (Android 7.0).
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.