tabbrowser.xml should use AppConstants from the global scope

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: dao, Assigned: Ganesh Chaitanya Kale, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 55
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
Instead of 'this.AppConstants' and 'this.tabbrowser.AppConstants', tabbrowser.xml should use the global 'AppConstants', and we should remove this field:

http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/browser/base/content/tabbrowser.xml#71-73
(Assignee)

Comment 1

5 months ago
Hey Dao,

I would like to work on this bug. Please let me know on how to proceed on this.

Regards,
Ganesh
(Reporter)

Comment 2

5 months ago
(In reply to ganesh2583 from comment #1)
> Hey Dao,
> 
> I would like to work on this bug. Please let me know on how to proceed on
> this.

The basic instructions are in the first comment. Is something unclear there?
(Assignee)

Comment 3

5 months ago
I understood the change that needs to be done. We have to remove the references in lines 71-73 and change the concurrences of this.AppConstants and this.tabbrowser.AppConstants to AppConstants. Let me know if its correct.
(Reporter)

Comment 4

5 months ago
That's correct.
(Assignee)

Comment 5

5 months ago
Created attachment 8850051 [details] [diff] [review]
Create the patch with the fix.

I have attached the patch. Please review and let me know if any further changes are required.
Attachment #8850051 - Flags: review?(dao+bmo)
(Reporter)

Comment 6

5 months ago
Comment on attachment 8850051 [details] [diff] [review]
Create the patch with the fix.

Looks good!

Try run to see if this causes test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=414c6ec60efc1ba057ffe461b7d0cd0f61d3c3ff
Attachment #8850051 - Flags: review?(dao+bmo) → review+
(Reporter)

Updated

5 months ago
Assignee: nobody → ganesh2583
(Assignee)

Comment 7

5 months ago
(In reply to Dão Gottwald [::dao] from comment #6)
> Comment on attachment 8850051 [details] [diff] [review]
> Create the patch with the fix.
> 
> Looks good!
> 
> Try run to see if this causes test failures:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=414c6ec60efc1ba057ffe461b7d0cd0f61d3c3ff

Hey DAO,

How can I run the tests, can you please let me know. I did a mach run and the Nightly Build Mozilla is up and I opened the link you have provided. If not sure on how to go forward after that.

Regards,
Ganesh
(Reporter)

Comment 8

5 months ago
The tests run automatically on the Try server. The test results appear under the link I posted as the tests finish.
(Assignee)

Comment 9

5 months ago
I executed the command ./mach mochitest -f browser and tests are running locally.
(Assignee)

Comment 10

5 months ago
I think there are no failures seeing the below link:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=414c6ec60efc1ba057ffe461b7d0cd0f61d3c3ff&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success
(Reporter)

Comment 11

5 months ago
Yep, looks good.

Comment 12

5 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d61e2f8bf8f7
Make tabbrowser.xml use AppConstants from the global scope. r=dao

Comment 13

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d61e2f8bf8f7
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.