Closed Bug 1377742 Opened 3 years ago Closed 3 years ago

about:rights shows XML parsing error

Categories

(Firefox for Android :: General, defect)

56 Branch
All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
fennec + ---
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 blocking verified
firefox57 + verified

People

(Reporter: gvanwaelvelde, Assigned: cnevinchen)

References

Details

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

Attachments

(2 files)

Attached image 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.
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 ?
Component: General → XML
Product: Firefox for Android → Core
Likely regressed by bug 1372005
Blocks: 1372005
tracking-fennec: --- → ?
Keywords: regression
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.
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.
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)
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: nobody → cnevinchen
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+
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c96f0963b33
about:rights shows XML parsing error. r=dao
tracking-fennec: ? → +
Flags: needinfo?(topwu.tw)
https://hg.mozilla.org/mozilla-central/rev/9c96f0963b33
Status: NEW → RESOLVED
Closed: 3 years ago
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.
Nevin, would you help check? tks.
Flags: needinfo?(wehuang) → needinfo?(cnevinchen)
Hi Dao.
Could you please teach me how to fix this?
Thanks!
Flags: needinfo?(cnevinchen) → needinfo?(dao+bmo)
(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
OS: Unspecified → Android
Hardware: Unspecified → All
Resolution: FIXED → ---
Duplicate of this bug: 1389659
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)
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
Closed: 3 years ago3 years 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 → ---
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
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 56 → Firefox 57
Please request Beta approval on this when you get a chance.
Flags: needinfo?(cnevinchen)
I've requested in bug 1390038 since the patch there fix the problem.
Flags: needinfo?(cnevinchen)
Verified as fixed on the latest Beta 56.0b5 build
Whiteboard: [FNC][SPT57.2][INT]
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
You need to log in before you can comment on or make changes to this bug.