Bug 1880582 Comment 85 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 Brad Werth [:bradwerth] from comment #82)
> I'm [not certain that your patch is sufficient and correct](https://hg.mozilla.org/try/rev/eb1a5b4b0331ef47698fc38bd8d77e44315750b1) because it will retain `mWindow` forever if `HideWindowChrome` is called.

Oops, I forgot about that. I'll revise my patch to deal with it.

> And syntactically it doesn't feel great to have an alloc-then-retain pattern.

I don't really see anything wrong with it. Feel free to come up with a new patch. But the basic problem is very simple, and so should the solution be. Don't encrust it with other stuff that may actually cause it to miss its goal.

I'll answer your questions in another comment.
(In reply to Brad Werth [:bradwerth] from comment #82)
> I'm [not certain that your patch is sufficient and correct](https://hg.mozilla.org/try/rev/eb1a5b4b0331ef47698fc38bd8d77e44315750b1) because it will retain `mWindow` forever if `HideWindowChrome` is called.

Oops, I forgot about that. I'll revise my patch to deal with it.

> And syntactically it doesn't feel great to have an alloc-then-retain pattern.

I don't really see anything wrong with it. Feel free to come up with a new patch. But the basic problem is very simple, and so should the solution be. Don't encrust it with other stuff that may actually cause it to miss its goal.

I'll answer your questions in another comment. (Actually they were Markus' questions.)

Back to Bug 1880582 Comment 85