Closed Bug 1496978 Opened Last year Closed Last year

"Show Title Bar" not shown in Customize Toolbar on Mac

Categories

(Thunderbird :: Toolbars and Tabs, defect)

All
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 64.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 1 obsolete file)

The "Show Title Bar" not shown in Customize Toolbar on Mac.

I get this error: TypeError: window.opener is null; can't access its "document" property. This is likely because on Mac the customizeToolbar isn't opened as window but as sheet.
Attached patch 1496978-showTitlebar.patch (obsolete) — Splinter Review
I don't know how we could detect it the sheet is opened from main window. Because of this, I added try/catch to stop the error and added the unhide to the CSS. With this the checkbox is only shown when opened from main view toolbar. When the sheet is opened on Chat or Calendar in the main window it isn't shown. I think, this is okay as such will be first done on main view.

Jörg, I know you can't test on Mac but the JS part should be enough to be sure I've done it correctly.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9015016 - Flags: review?(jorgk)
Hmm, were do you get the error? In the code we have
  if (window.opener && ...
so that doesn't run when window.opener is null. Does adding the 'try' really help, I don't think it fails there.

Maybe in some other place we need to add the same check.
(In reply to Jorg K (GMT+2) from comment #2)
> Hmm, were do you get the error? In the code we have
>   if (window.opener && ...
> so that doesn't run when window.opener is null. Does adding the 'try' really
> help, I don't think it fails there.
> 
> Maybe in some other place we need to add the same check.

Here is the full text:
TypeError: window.opener is null; can't access its "document" property[Learn More] customizeToolbar.js:87:1

I found a second (window.opener && ... here: https://searchfox.org/comm-central/source/mail/base/content/mailTabs.js#72
Hmm, that makes no sense. This is the code:
    if (window.opener &&
87     (window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane")) {

Something is fishy here, how can it evaluate the second expression, if the first one is already falsy??? Is this a JS bug? What's going on here?

Aceman?
Flags: needinfo?(acelists)
Comment on attachment 9015016 [details] [diff] [review]
1496978-showTitlebar.patch

Tooru-san, how is it possible that the code quoted in comment #4 produces:
  TypeError: window.opener is null; can't access its "document" property.
Looks like a JS bug to me.

Richard, a few questions:
- This can only be tested on Mac, yes?
- Can you please try a few variations.

Existing:
    if (window.opener &&
       (window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane")) {

Variation 1:
    if (!!window.opener &&
       (window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane")) {

Variation 2:
    if (window.opener &&
        window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane") {

Variation 3:
    if (!!window.opener &&
        window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane") {

Variation 4:
    if (window.opener)
      if (window.opener.document.documentElement.getAttribute("windowtype") == "mail:3pane") {
        ...
      }

Really strange, or am I missing something? Clearing the review for now since papering over the problem with a try/catch is not the right approach.
Attachment #9015016 - Flags: review?(jorgk)
The strange thing is, I don't see the error any more with the today Daily.

I can't really test as my self built TB is completely black because of the 10.14 SDK.

Would you be okay when I make a patch with only the CSS change?
Sure, but I'd also like to understand the JS problem. The error on the console wasn't a ghost.
Okay, only CSS.

Yes it's really strange with the error.
Attachment #9015016 - Attachment is obsolete: true
Attachment #9015064 - Flags: review?(jorgk)
Comment on attachment 9015064 [details] [diff] [review]
1496978-showTitlebar.patch

I'm the wrong reviewer for Mac stuff. This had better be working ;-)
Attachment #9015064 - Flags: review?(jorgk) → review+
The only Mac reviewer seems to be Philipp and he's not around.

With my SDK issue I can't also not really develop on Mac.
Flags: needinfo?(acelists)
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0a609462906f
Show the #showTitlebar checkbox on Mac too. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
Arai, please see comment 5.
Flags: needinfo?(arai.unmht)
Tried with today Daily, no error in console.
I cannot reproduce the error message on 64.0a1 (2018-10-06) (64-bit) on macOS 10.13.6.
`window.opener` is evaluated to be falsy.
Flags: needinfo?(arai.unmht)
You need to log in before you can comment on or make changes to this bug.