Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Multiple errors are thrown while opening and closing Firefox Screenshots #3097

Closed
emilghittasv opened this issue Jul 10, 2017 · 9 comments
Closed
Assignees
Labels
firefox Something that has to ship in the add-on in Firefox P1
Milestone

Comments

@emilghittasv
Copy link
Collaborator

[Affected versions]:
Firefox 55.0b7 (Build Id:20170706085221)
Firefox 56.0a1 (Build Id:20170709030204)
Firefox Screenshots version: 10.3.0

[Affected platforms]:
Windows 10 64 bit
macOS 10.11.6
Ubuntu 16.04 64bit

[Steps to reproduce]:

  1. Launch Firefox with a clean profile.
  2. Enable Firefox Screenshots.
  3. Open Firefox Screenshots.
  4. Skip the Firefox Screenshots tutorial.
  5. Close Firefox Screenshots
  6. Access the http://www.bbc.com/ webpage.
  7. Click on any available article.
  8. Immediately click the Firefox Screenshots button several times and fast (while the article loads and after the article has been successfully loaded).

[Expected result]:
Firefox Screenshots opens and closes without any errors.

[Actual result]:
The following errors are constantly displayed:

Windows 10 and Ubuntu 16.04
"No matching message handler"
"catcher is not defined"
"ui is not defined"

macOS 10.11.6:
"We can't screenshot this page" is constantly thrown even if the page was successfully loaded. Also the "catcher is not defined" error is thrown in the console.

[Additional notes]
Please note that this issue is not reproducible at all times (you may need to repeat steps 7 and 8).
Also please note that sometimes the Firefox Screenshots layer remains permanently on the screen and if you try to close/open Firefox Screenshots the "ui is not defined error is constantly thrown".

For further information regarding this issue please observe the following video: https://goo.gl/VLtRgY

@ghost ghost added this to the General Release 55 milestone Jul 10, 2017
@emilghittasv
Copy link
Collaborator Author

I just want to add that I encountered the "Shot is not defined" error while following mainly the up mentioned steps on the https://www.reddit.com/ webpage.

This error message pops up at random times when you press the Firefox Screenshots button several times while the page is loading.

For further information please observe the following video: https://goo.gl/4EC71u

@johngruen
Copy link
Contributor

johngruen commented Jul 13, 2017

Ritu mentioned this in the release drivers email list:

I can repro the bbc.com screenshots issue on latest Nightly56 pretty consistently. There is no fuzzing involved. It's a simple STR: 1) Open a new tab, 2) go to bbc.com, 3) While the page is loading, click on screenshots icon, 4) Click on "save visible". This is the screenshot URL I get https://screenshots.firefox.com/BHNu15yUmAtfTxeY/www.bbc.com and there is a spinner in the middle of that tab as if it's busy doing something or in a hang state.

However, it seems this issue may be a side effect of https://bugzilla.mozilla.org/show_bug.cgi?id=1380507

@SoftVision-PaulOiegas Since you're looking at Screenshots for the next few days can you try to repro?

@jaredhirsch jaredhirsch self-assigned this Jul 14, 2017
@jaredhirsch
Copy link
Member

jaredhirsch commented Jul 14, 2017

I tried adding some console.logs to the start and end of catcher.js and ui.js.

If you hit the button over and over, very quickly, it looks like the different files are loaded in non-deterministic order. In the cases where required scripts are missing dependencies, sadness occurs.

The loadIfNecessary code assembles a chain of Promises, but doesn't cancel them when the overlay UI is torn down, so I'm not sure if that could be related here.

screen shot 2017-07-14 at 2 08 25 pm

@jaredhirsch
Copy link
Member

The simplest fix here might be to concat all the lazy-loaded scripts in our build process

@ianb
Copy link
Contributor

ianb commented Jul 14, 2017

If different files are being loaded in non-deterministic order, I can only imagine that's because selectorLoader.loadModules is running multiple times and the loads are interleaved. I think we could just block loadModules so it can't be run more than once at a time (taking into account it's an async function).

@ianb
Copy link
Contributor

ianb commented Aug 21, 2017

Note also #3222 (comment)

ianb added a commit that referenced this issue Aug 22, 2017
Note: source maps don't actually work yet, for unknown reasons
@jaredhirsch
Copy link
Member

It turns out this can still be reproduced in 57+ if the page action is added to the address bar, then the user clicks the page action rapidly.

But note that, by default, the Screenshots item is buried in the page action menu, where it can't be rapidly toggled. Maybe this should be a P2?

@ianb
Copy link
Contributor

ianb commented Sep 20, 2017

I opened Bug 1401241 about the sourcemap issue with executeScript.

@ghost ghost added the firefox Something that has to ship in the add-on in Firefox label Sep 21, 2017
@ghost ghost modified the milestones: General Release 57, Launch 58 Sep 21, 2017
@ghost
Copy link

ghost commented Sep 21, 2017

duplicate of #2983

@ghost ghost closed this as completed Sep 21, 2017
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firefox Something that has to ship in the add-on in Firefox P1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants