Bug 1786988 Comment 83 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Wayne Mery (:wsmwk) from comment #81)
> Thomas, can you tackle comment 80?

Ok, this was a bit of a hassle for a staunch Windows user and Mac newcomer like myself. However ultimately, looks like I just kept shooting myself in the foot by forgetting to use `-purgecaches` when tweaking that single line of MsgComposeCommands.js in omni.ja. On the upside, I now have Visual Studio Code installed on my Mac :-)


(In reply to Magnus Melin [:mkmelin] from comment #80)
> (In reply to Wayne Mery (:wsmwk) from comment #78)
> > I doubt we need any further testing to show that this began with bug 1774123, or another patch in the immediate vicinity of June 17.
> Right. It needs someone with a Mac to debug what's going on here, backing out 9fd8ef760060 locally and checking how the preferences play into it all.

Here's my findings:
- backing out [9fd8ef760060](https://phabricator.services.mozilla.com/rCOMMCENTRAL9fd8ef76006078fd50b8470e3590bcebbe62638a) from bug 1774123 locally fixes the problem of this bug (disabled help menu) regardless of the pref settings for `mailnews.show_send_progress` or `mailnews.sendInBackground`. The backout reverts `"true"` to `true` in [this line](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1658).
- Setting `mailnews.sendInBackground = true` has the same net effect as setting `mailnews.show_send_progress = false`: both will prevent showing the send progress dialogue. It seems to be presence of the Send Progress dialogue which makes this fail when the above changeset isn't backed out.
- It's worth noting that Mac only ever shows a single main menu, which is different from Windows where each window (also composition) has its own main menu. So maybe the dialog getting focus somehow interferes with us getting to that menu.
- The associated functions like [setDisabledState](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1630-1638) need some scrutiny around `"true"` (string)  vs. `true` (boolean) and logic of the conditionals:

```
  function setDisabledState(aElement, aValue) {
    if ("disabled" in aElement) {
      aElement.disabled = aValue == "true";
    } else if (aValue == "") {
      aElement.removeAttribute("disabled");
    } else {
      aElement.setAttribute("disabled", aValue);
    }
  }
```

Imo, the `else if` is no-op, because you can only remove a disabled attribute which exists, but if it exists, we'll never get here because the `if("disabled" in aElement)` will catch that first.
(In reply to Wayne Mery (:wsmwk) from comment #81)
> Thomas, can you tackle comment 80?

Ok, this was a bit of a hassle for a staunch Windows user and Mac newcomer like myself. However ultimately, looks like I just kept shooting myself in the foot by forgetting to use `-purgecaches` when tweaking that single line of MsgComposeCommands.js in omni.ja. On the upside, I now have 'Visual Studio Code' installed on my Mac :-)


(In reply to Magnus Melin [:mkmelin] from comment #80)
> (In reply to Wayne Mery (:wsmwk) from comment #78)
> > I doubt we need any further testing to show that this began with bug 1774123, or another patch in the immediate vicinity of June 17.
> Right. It needs someone with a Mac to debug what's going on here, backing out 9fd8ef760060 locally and checking how the preferences play into it all.

Here's my findings:
- backing out [9fd8ef760060](https://phabricator.services.mozilla.com/rCOMMCENTRAL9fd8ef76006078fd50b8470e3590bcebbe62638a) from bug 1774123 locally fixes the problem of this bug (disabled help menu) regardless of the pref settings for `mailnews.show_send_progress` or `mailnews.sendInBackground`. The backout reverts `"true"` to `true` in [this line](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1658).
- Setting `mailnews.sendInBackground = true` has the same net effect as setting `mailnews.show_send_progress = false`: both will prevent showing the send progress dialogue. It seems to be presence of the Send Progress dialogue which makes this fail when the above changeset isn't backed out.
- It's worth noting that Mac only ever shows a single main menu, which is different from Windows where each window (also composition) has its own main menu. So maybe the dialog getting focus somehow interferes with us getting to that menu.
- The associated functions like [setDisabledState](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1630-1638) need some scrutiny around `"true"` (string)  vs. `true` (boolean) and logic of the conditionals:

```
  function setDisabledState(aElement, aValue) {
    if ("disabled" in aElement) {
      aElement.disabled = aValue == "true";
    } else if (aValue == "") {
      aElement.removeAttribute("disabled");
    } else {
      aElement.setAttribute("disabled", aValue);
    }
  }
```

Imo, the `else if` is no-op, because you can only remove a disabled attribute which exists, but if it exists, we'll never get here because the `if("disabled" in aElement)` will catch that first.
(In reply to Wayne Mery (:wsmwk) from comment #81)
> Thomas, can you tackle comment 80?

Ok, this was a bit of a hassle for a staunch Windows user and Mac newcomer like myself. However ultimately, looks like I just kept shooting myself in the foot by forgetting to use `-purgecaches` when tweaking that single line of MsgComposeCommands.js in omni.ja. On the upside, I now have 'Visual Studio Code' installed on my Mac :-)


(In reply to Magnus Melin [:mkmelin] from comment #80)
> (In reply to Wayne Mery (:wsmwk) from comment #78)
> > I doubt we need any further testing to show that this began with bug 1774123, or another patch in the immediate vicinity of June 17.
> Right. It needs someone with a Mac to debug what's going on here, backing out 9fd8ef760060 locally and checking how the preferences play into it all.

Here's my findings:
- backing out [9fd8ef760060](https://phabricator.services.mozilla.com/rCOMMCENTRAL9fd8ef76006078fd50b8470e3590bcebbe62638a) from bug 1774123 locally fixes the problem of this bug (disabled help menu) regardless of the pref settings for `mailnews.show_send_progress` or `mailnews.sendInBackground`. The backout reverts `"true"` to `true` in [this line](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1658).
- Setting `mailnews.sendInBackground = true` has the same net effect as setting `mailnews.show_send_progress = false`: both will prevent showing the send progress dialogue. It seems to be presence of the Send Progress dialogue which makes this fail when the above changeset isn't backed out.
- It's worth noting that Mac only ever shows a single main menu, which is different from Windows where each window (also composition) has its own main menu. So maybe the dialog getting focus somehow interferes with us getting to that generic menu.
- The associated functions like [setDisabledState](https://searchfox.org/comm-central/rev/676fb4c0030ea6ecdebf864a79b817567d60640e/mail/components/compose/content/MsgComposeCommands.js#1630-1638) need some scrutiny around `"true"` (string)  vs. `true` (boolean) and logic of the conditionals:

```
  function setDisabledState(aElement, aValue) {
    if ("disabled" in aElement) {
      aElement.disabled = aValue == "true";
    } else if (aValue == "") {
      aElement.removeAttribute("disabled");
    } else {
      aElement.setAttribute("disabled", aValue);
    }
  }
```

Imo, the `else if` is no-op, because you can only remove a disabled attribute which exists, but if it exists, we'll never get here because the `if("disabled" in aElement)` will catch that first.

Back to Bug 1786988 Comment 83