Issue 3486 - Issue reporter: don`t access content window to reload the page

Issue 3486 - Issue reporter: don`t access content window to reload the page

[Erik, 2016-01-08 03:56:48]

LGTM

[Erik, 2016-01-08 04:00:46]

https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
File chrome/content/ui/sendReport.js (right):

https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
chrome/content/ui/sendReport.js:812: this.browser.reload();
the only problem I see with this is that the current tab might change and this
would reload a different window than previously.  Also it might be better to
avoid holding a ref to browser somehow, but I don't see an issue with it for
now.

[Wladimir Palant, 2016-01-08 13:46:20]

https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
File chrome/content/ui/sendReport.js (right):

https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
chrome/content/ui/sendReport.js:812: this.browser.reload();
On 2016/01/08 04:00:45, Erik wrote:
> the only problem I see with this is that the current tab might change and this
> would reload a different window than previously.

Given that it is a specific <browser> element rather than <tabbrowser> - we will
always reload the right tab. It might be that a different page was loaded into
this tab but reloading it isn't a big issue IMHO.

> Also it might be better to
> avoid holding a ref to browser somehow, but I don't see an issue with it for
> now.

You mean - going through the opener's tabs looking for the browser with a
specific outerWindowID? That would be a possibility but hard to implement across
apps (in particular on Thunderbird - we will have <tabmail> there). This use
case doesn't seem to justify the complexity.

[Erik, 2016-01-08 22:15:36]

On 2016/01/08 13:46:20, Wladimir Palant wrote:
>
https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
> File chrome/content/ui/sendReport.js (right):
> 
>
https://codereview.adblockplus.org/29333312/diff/29333313/chrome/content/ui/s...
> chrome/content/ui/sendReport.js:812: this.browser.reload();
> On 2016/01/08 04:00:45, Erik wrote:
> > the only problem I see with this is that the current tab might change and
this
> > would reload a different window than previously.
> 
> Given that it is a specific <browser> element rather than <tabbrowser> - we
will
> always reload the right tab. It might be that a different page was loaded into
> this tab but reloading it isn't a big issue IMHO.

Ah I was confused about that, you are right.
 
> > Also it might be better to
> > avoid holding a ref to browser somehow, but I don't see an issue with it for
> > now.
> 
> You mean - going through the opener's tabs looking for the browser with a
> specific outerWindowID? That would be a possibility but hard to implement
across
> apps (in particular on Thunderbird - we will have <tabmail> there). This use
> case doesn't seem to justify the complexity.

Once we have a function to do this it isn't so bad to use it, although there
might be a better way that we could discover later, anyhow it's not a blocker.