Last Comment Bug 819907 - add permanent fullsc-toggler element to simplify code
: add permanent fullsc-toggler element to simplify code
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: 20 Branch
: All All
: -- normal (vote)
: Firefox 20
Assigned To: ithinc
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-10 05:36 PST by ithinc
Modified: 2012-12-26 05:20 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (6.79 KB, patch)
2012-12-15 15:54 PST, ithinc
gavin.sharp: review+
Details | Diff | Splinter Review
patch v2 (9.66 KB, patch)
2012-12-15 16:15 PST, ithinc
no flags Details | Diff | Splinter Review
patch for checkin (as v1) (7.04 KB, patch)
2012-12-24 06:50 PST, ithinc
no flags Details | Diff | Splinter Review

Description ithinc 2012-12-10 05:36:24 PST
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.
Comment 1 ithinc 2012-12-15 15:54:23 PST
Created attachment 692668 [details] [diff] [review]
patch
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 16:07:14 PST
Comment on attachment 692668 [details] [diff] [review]
patch

Looks good to me!
Comment 3 ithinc 2012-12-15 16:15:20 PST
Created attachment 692672 [details] [diff] [review]
patch v2
Comment 4 ithinc 2012-12-16 03:42:25 PST
Comment on attachment 692672 [details] [diff] [review]
patch v2

Replace _isChromeCollapsed with !_fullScrToggler.collapsed.
Comment 5 ithinc 2012-12-24 06:50:43 PST
Created attachment 695472 [details] [diff] [review]
patch for checkin (as v1)
Comment 6 ithinc 2012-12-24 06:54:20 PST
"Replace _isChromeCollapsed with !_fullScrToggler.collapsed" will be handled in a followup.
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-12-24 12:30:20 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/d83930c9a712
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-12-26 05:20:48 PST
https://hg.mozilla.org/mozilla-central/rev/d83930c9a712

Note You need to log in before you can comment on or make changes to this bug.