Closed Bug 1022214 Opened 6 years ago Closed 6 years ago

Remove the obsolete mTabstripCloseButton property

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 33

People

(Reporter: quicksaver, Assigned: shashank)

References

Details

(Whiteboard: [good first bug][lang=xul][mentor=dao] p=0 s=33.1 [qa-])

Attachments

(1 file)

Property .mTabstripCloseButton in #tabbrowser-tabs always returns null now. http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#41541
Flags: firefox-backlog+
Summary: Dead code after bug 865826 → Remove the obsolete mTabstripCloseButton property
Whiteboard: [good first bug][lang=xul][mentor=dao]
Version: 31 Branch → Trunk
(In reply to Luís Miguel [:Quicksaver] from comment #0)
> Property .mTabstripCloseButton in #tabbrowser-tabs always returns null now.
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> tabbrowser.xml#41541

Are you referring to http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#4160 ?

Is it just enough to delete this property or the next one too?
Flags: needinfo?(quicksaver)
Yes it's that, I'm sorry about that, must have been a copy-paste mishap, didn't even notice it.

mTabStripClosebutton returns a node with id "tabs-closebutton" which was removed by bug 865826. A search in central for "tabs-closebutton" and "mTabStripClosebutton" return only that single line. So that property will return null at all times when called, which it never is apparently.

Perhaps I'm missing a specific case as to why this variable was kept (needinfo'ing Dao on this as he was assigned that bug and will probably know the reason if there is any), but I don't see how this helps/hurts with any backwards compatibility issues, whether it is in-house or for add-ons.

I don't know exactly what the use of mAllTabsPopup is/was, so I can't answer that. But I'm sure that is not the same case as mTabStripCloseButton, as the node it refers to still exists at least (from the name I think it's the popup menu that opens when you click the all tabs button, haven't investigated further than that though).
Flags: needinfo?(quicksaver) → needinfo?(dao)
(In reply to Shashank VRSN Sabniveesu from comment #1)
> (In reply to Luís Miguel [:Quicksaver] from comment #0)
> > Property .mTabstripCloseButton in #tabbrowser-tabs always returns null now.
> > http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> > tabbrowser.xml#41541
> 
> Are you referring to
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> tabbrowser.xml#4160 ?
> 
> Is it just enough to delete this property or the next one too?

Only the mTabstripClosebutton property.
Flags: needinfo?(dao)
Assignee: nobody → shashank
Attachment #8437983 - Flags: review?(dao)
Flags: needinfo?(dao)
Comment on attachment 8437983 [details] [diff] [review]
BUG1022214.patch Removed the relevant 'property' element

Thanks!
Attachment #8437983 - Flags: review?(dao) → review+
Flags: needinfo?(dao)
https://hg.mozilla.org/mozilla-central/rev/20f9b816ebcd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Whiteboard: [good first bug][lang=xul][mentor=dao] → [good first bug][lang=xul][mentor=dao] p=0 s=33.1 [qa?]
Whiteboard: [good first bug][lang=xul][mentor=dao] p=0 s=33.1 [qa?] → [good first bug][lang=xul][mentor=dao] p=0 s=33.1 [qa-]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.