If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove remnants of Browser Debugger

RESOLVED FIXED in Firefox 50

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: past, Assigned: dalimil, Mentored)

Tracking

Trunk
Firefox 50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

Once upon a time in a land far far away, there was a Browser Debugger in devtools. Seasons came and seasons went and the Browser Debugger grew up to become the Browser Toolbox. The Browser Debugger had debugger.xul as the sole document in the window and updated the window title in debugger-panes.js:_onSourceSelect. Still to this day browser.properties contains the remains of that era, the strings DebuggerWindowTitle and DebuggerWindowScriptTitle.

Since the Browser Toolbox window contains the toolbox.xul document with debugger.xul as an iframe, the Old Strings can no longer be seen and are forever cursed to live in the catacombs of the Code Repository. The entire world craves for a valiant knight in his white horse who will slay these dragons and liberate the debugger source from the iron grip of The Ancients, once and for all.

May you, gentle prince, be the one they are waiting for?
Mentor: jryans@gmail.com
Whiteboard: [good first bug]
(Assignee)

Comment 1

a year ago
I would like to work on this. Can someone assign this to me?
(Assignee)

Comment 2

a year ago
Created attachment 8762591 [details] [diff] [review]
rev1

I removed the strings, although I wasn't 100% it is safe to remove the code setting the document.title in sources-view.js and debugger-view.js
Attachment #8762591 - Flags: review?(jryans)
Comment on attachment 8762591 [details] [diff] [review]
rev1

Review of attachment 8762591 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me!  Sorry for the delay, all of Mozilla is away at a meeting this week.

Could you update the commit message to say the reviewer as "r=jryans" instead?

Once you post the updated patch, I can send it to our try server for testing.
Attachment #8762591 - Flags: review?(jryans) → feedback+
(Assignee)

Comment 4

a year ago
Created attachment 8763039 [details] [diff] [review]
rev2
Attachment #8762591 - Attachment is obsolete: true
Attachment #8763039 - Flags: review?(jryans)
Assignee: nobody → dalimilhajek
Status: NEW → ASSIGNED
Comment on attachment 8763039 [details] [diff] [review]
rev2

Review of attachment 8763039 [details] [diff] [review]:
-----------------------------------------------------------------

Great, sorry again for the delay.  Thanks for working on this!

I'll submit this to the try server now.  Assuming the results look good, you can add "checkin-needed" to the keywords field of the bug.  (I gave your account access to do so.)

Please check out http://firefox-dev.tools for other bugs you may be interested in.
Attachment #8763039 - Flags: review?(jryans) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b1e4644fb4d
From the try failures, it looks like we also need to update the test devtools/client/debugger/test/mochitest/browser_dbg_scripts-switching-01.js, which was checking the title.

You can run the test locally with:

./mach mochitest devtools/client/debugger/test/mochitest/browser_dbg_scripts-switching-01.js

Could you take a look at this?
Flags: needinfo?(dalimilhajek)
(Assignee)

Comment 8

a year ago
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> From the try failures, it looks like we also need to update the test
> devtools/client/debugger/test/mochitest/browser_dbg_scripts-switching-01.js,
> which was checking the title.
> 
> You can run the test locally with:
> 
> ./mach mochitest
> devtools/client/debugger/test/mochitest/browser_dbg_scripts-switching-01.js
> 
> Could you take a look at this?

Yes, I'll take a look at it in the next few days.
Flags: needinfo?(dalimilhajek)
(Assignee)

Comment 9

a year ago
Created attachment 8765109 [details] [diff] [review]
rev3

Ok, I fixed it.
Attachment #8763039 - Attachment is obsolete: true
Attachment #8765109 - Flags: review?(jryans)
Comment on attachment 8765109 [details] [diff] [review]
rev3

Review of attachment 8765109 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, let's see what try thinks:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=875f3f4ea510
Attachment #8765109 - Flags: review?(jryans) → review+
Great, the try run looks good.  One of the sheriffs will land this to fx-team soon, and then merge it to mozilla-central a day later.  The bug will be marked resolved once it merges.

Thanks for working on this!
Keywords: checkin-needed

Comment 12

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/4535711555f7
Remove remnants of Browser Debugger. r=jryans
Keywords: checkin-needed

Comment 13

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4535711555f7
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.