Bug 1565007 Comment 27 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 Michael Weghorn from comment #26)
> I have now submitted a rebased and updated version of the patch ( https://phabricator.services.mozilla.com/D76327 )  that tries to address the suggestions (uses 'let' instead of 'var', capitalized comments). I'd be happy to hear if any other changes should be made.

Thank you very much, Michael! This seems to fix the regression without any adverse effects and without touching the current focus patterns. Let's hear from Magnus if we can roll with this.

Magnus, could you please have another look in view of my comment 22?

(In reply to Thomas D. (:thomas8) from comment #22)
> In duplicate bug 1607695, new contributor Michael W. had provided a patch some months ago, which imo fixes this bug quite elegantly.
> Magnus said r- citing focus problems. Assignee had questions on the review which unfortunately hadn't received a reply until now.
> 
> I have rebased and tested the patch locally (not yet attached here), and it still works perfectly, exactly as expected. For certain click patterns involving the Save button on attachment header, indeed focus is force-changed to the message list a bit prematurely as the slower file picker opens async and is overtaken by the subsequent focus change. However, that minor disorder has been there before the patch, and it doesn't seem to cause any problems as save dialogs are always correctly focused, so we can still polish that in a follow-up bug if we really want.
> 
> I suggest to rebase this [done] and land essentially as-is, with some nits like var vs. let and comment capitalization [done].
(In reply to Michael Weghorn from comment #26)
> I have now submitted a rebased and updated version of the patch ( https://phabricator.services.mozilla.com/D76327 )  that tries to address the suggestions (uses 'let' instead of 'var', capitalized comments). I'd be happy to hear if any other changes should be made.

Thank you very much, Michael! This seems to fix the regression without any adverse effects on the current focus patterns, which remain the same before and after the patch. Let's hear from Magnus if we can roll with this.

Magnus, could you please have another look in view of my comment 22?

(In reply to Thomas D. (:thomas8) from comment #22)
> In duplicate bug 1607695, new contributor Michael W. had provided a patch some months ago, which imo fixes this bug quite elegantly.
> Magnus said r- citing focus problems. Assignee had questions on the review which unfortunately hadn't received a reply until now.
> 
> I have rebased and tested the patch locally (not yet attached here), and it still works perfectly, exactly as expected. For certain click patterns involving the Save button on attachment header, indeed focus is force-changed to the message list a bit prematurely as the slower file picker opens async and is overtaken by the subsequent focus change. However, that minor disorder has been there before the patch, and it doesn't seem to cause any problems as save dialogs are always correctly focused, so we can still polish that in a follow-up bug if we really want.
> 
> I suggest to rebase this [done] and land essentially as-is, with some nits like var vs. let and comment capitalization [done].
(In reply to Michael Weghorn from comment #26)
> I have now submitted a rebased and updated version of the patch ( https://phabricator.services.mozilla.com/D76327 )  that tries to address the suggestions (uses 'let' instead of 'var', capitalized comments). I'd be happy to hear if any other changes should be made.

Thank you very much, Michael! This seems to fix the regression without any adverse effects on the current focus patterns, which remain the same before and after the patch. Let's hear from Magnus if we can roll with this.

Magnus, could you please have another look in view of my comment 22?

(In reply to Thomas D. (:thomas8) from comment #22)
> In duplicate bug 1607695, new contributor Michael W. had provided a patch some months ago, which imo fixes this bug quite elegantly.
> Magnus said r- citing focus problems. Assignee had questions on the review which unfortunately hadn't received a reply until now.
> 
> I have rebased and tested the patch locally [now attached here by assignee], and it still works perfectly, exactly as expected. For certain click patterns involving the Save button on attachment header, indeed focus is force-changed to the message list a bit prematurely as the slower file picker opens async and is overtaken by the subsequent focus change. However, that minor disorder has been there before the patch, and it doesn't seem to cause any problems as save dialogs are always correctly focused, so we can still polish that in a follow-up bug if we really want.
> 
> I suggest to rebase this [done] and land essentially as-is, with some nits like var vs. let and comment capitalization [done].

Back to Bug 1565007 Comment 27