Closed
Bug 819907
Opened 13 years ago
Closed 13 years ago
add permanent fullsc-toggler element to simplify code
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
Firefox 20
People
(Reporter: tabutils+bugzilla, Assigned: tabutils+bugzilla)
Details
Attachments
(1 file, 2 obsolete files)
|
7.04 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121204 Firefox/20.0
Build ID: 20121204030754
Steps to reproduce:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullScreen.js#49
49 let fullScrToggler = document.getElementById("fullscr-toggler");
50 if (!fullScrToggler) {
51 fullScrToggler = document.createElement("hbox");
52 fullScrToggler.id = "fullscr-toggler";
53 fullScrToggler.collapsed = true;
54 gNavToolbox.parentNode.insertBefore(fullScrToggler, gNavToolbox.nextSibling);
55 }
56 fullScrToggler.addEventListener("mouseover", this._expandCallback, false);
57 fullScrToggler.addEventListener("dragenter", this._expandCallback, false);
58 }
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullScreen.js#157
157 let fullScrToggler = document.getElementById("fullscr-toggler");
158 if (fullScrToggler) {
159 fullScrToggler.removeEventListener("mouseover", this._expandCallback, false);
160 fullScrToggler.removeEventListener("dragenter", this._expandCallback, false);
161 }
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullScreen.js#173
173 let fullScrToggler = document.getElementById("fullscr-toggler");
174 if (fullScrToggler) {
175 fullScrToggler.removeEventListener("mouseover", this._expandCallback, false);
176 fullScrToggler.removeEventListener("dragenter", this._expandCallback, false);
177 }
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullScreen.js#509
509 let toggler = document.getElementById("fullscr-toggler");
510 if (toggler) {
511 toggler.collapsed = aShow;
512 }
Expected results:
The "fullscr-toggler" would be better constructed in browser.xul and cached in FullScreen object.
Updated•13 years ago
|
Component: Untriaged → Toolbars
Attachment #692668 -
Flags: review?(felipc)
Attachment #692668 -
Flags: feedback?(gavin.sharp)
Comment 2•13 years ago
|
||
Comment on attachment 692668 [details] [diff] [review]
patch
Looks good to me!
Attachment #692668 -
Flags: review?(felipc)
Attachment #692668 -
Flags: review+
Attachment #692668 -
Flags: feedback?(gavin.sharp)
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Refactor browser-fullScreen.js a little to simplify code → add permanent fullsc-toggler element to simplify code
Updated•13 years ago
|
Assignee: nobody → ithinc
OS: Windows 7 → All
Hardware: x86_64 → All
Attachment #692668 -
Attachment is obsolete: true
Attachment #692672 -
Flags: review?(felipc)
Attachment #692672 -
Flags: feedback?(gavin.sharp)
Comment on attachment 692672 [details] [diff] [review]
patch v2
Replace _isChromeCollapsed with !_fullScrToggler.collapsed.
Attachment #692672 -
Flags: review?(gavin.sharp)
Attachment #692672 -
Flags: review?(felipc)
Attachment #692672 -
Flags: feedback?(gavin.sharp)
Attachment #692672 -
Attachment is obsolete: true
Attachment #692672 -
Flags: review?(gavin.sharp)
"Replace _isChromeCollapsed with !_fullScrToggler.collapsed" will be handled in a followup.
Keywords: checkin-needed
Comment 7•13 years ago
|
||
Keywords: checkin-needed
Comment 8•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in
before you can comment on or make changes to this bug.
Description
•