Closed Bug 1669666 Opened 4 years ago Closed 3 years ago

Browsing-context actor's onWindowCreated handler gets invoked for `pageshow` events dispatched by wizard.js and it gets confused

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox83 wontfix, firefox84 verified)

VERIFIED FIXED
84 Branch
Tracking Status
firefox83 --- wontfix
firefox84 --- verified

People

(Reporter: csasca, Assigned: jdescottes)

References

Details

Attachments

(3 files)

Attached image Import Wizard.PNG

Affected versions

  • Firefox 82.0b8
  • Firefox 83.0a1

Affected platforms

  • Windows 7 & 10
  • Ubuntu 18.04
  • macOS 10.15

Steps to reproduce

  1. Launch Firefox
  2. Access Library
  3. Select "Import and Backup" and click on "Import Data from Another Browser"

Expected result

  • No errors are shown

Actual result

  • "TypeError: window is undefined" and some details below it are thrown in Browser Console

Regression range

  • I will see for a regression, if there is one.

Additional notes

  • The issue can be seen in the attachment

Suggested severity

  • S3-S4
Has Regression Range: --- → no
Has STR: --- → yes
Component: Migration → General
Product: Firefox → DevTools
Summary: Error is thrown in Browser Console when accessing Import Wizard → Error is thrown in Browser Console when accessing the Import Wizard if the browser console is open
Summary: Error is thrown in Browser Console when accessing the Import Wizard if the browser console is open → Error is thrown by browsing-context actor's onWindowCreated handler when accessing the Import Wizard if the browser console is open

This tripped errors at least on the 2019-01-01 build, so it goes back quite a while. No errors appear if you open the browser console after opening the library.

Looks like this is just a result of the wizard using pagehide and pageshow events, which are very different from website pagehide and pageshow events and so the debugging code gets very confused.

"But Gijs, why does the toolkit wizard code use those event names, that's dumb, aren't they clearly used by websites? Why not have different event names?

The wizard event names probably definitely predate the page visibility events spec.

We could (should?) change them but it'd be "fun" updating all the other consumers (also in comm-central etc.) - and in context, nobody is likely to get confused. It's just the devtools code because it listens for these events everywhere...

Summary: Error is thrown by browsing-context actor's onWindowCreated handler when accessing the Import Wizard if the browser console is open → Browsing-context actor's onWindowCreated handler gets invoked for `pageshow` events dispatched by wizard.js and it gets confused

Alex, I see that you've made these changes as part of the work on Bug 1378133.
Can you please take a look, thanks.

Flags: needinfo?(poirot.alex)
See Also: → 1675448
Assignee: nobody → jdescottes
Severity: -- → S3
Status: NEW → ASSIGNED
Flags: needinfo?(poirot.alex)
Priority: -- → P3
Blocks: 1650679
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ffd92ffe099
[devtools] Ignore custom pageshow/pagehide events in BrowsingContextTargetActor r=ochameau
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
Flags: qe-verify+

(In reply to Pulsebot from comment #5)

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ffd92ffe099
[devtools] Ignore custom pageshow/pagehide events in
BrowsingContextTargetActor r=ochameau

== Change summary for alert #27737 (as of Thu, 19 Nov 2020 04:46:28 GMT) ==

Regressions:

Ratio Suite Test Platform Options Absolute values (old vs new)
10% raptor-tp6-tumblr-firefox-cold fcp windows10-64-shippable-qr webrender 650.71 -> 717.92
4% raptor-tp6-microsoft-firefox-cold fcp windows10-64-shippable 502.75 -> 521.75

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
3% raptor-tp6-microsoft-firefox-cold fcp windows10-64-shippable 515.88 -> 502.17

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=27737

(In reply to Alexandru Ionescu (needinfo me) [:alexandrui] from comment #7)

(In reply to Pulsebot from comment #5)

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ffd92ffe099
[devtools] Ignore custom pageshow/pagehide events in
BrowsingContextTargetActor r=ochameau

== Change summary for alert #27737 (as of Thu, 19 Nov 2020 04:46:28 GMT) ==

How sure are you about this? This patch only affected browser-level devtools, which I would not expect to be running at all for raptor tests.

Flags: needinfo?(aionescu)

Well, tumblr one is definetely not caused by your patch, I unlinked it. The other is possibly infra change, so I did some retriggers. Thanks for the headsup!

Flags: needinfo?(aionescu)
Attached image error

Reproduced the issue with Firefox 83.0a1 (20201007155036) on Windows 10x64.
The issue is verified fixed with Firefox 84.0b4(20201122152513) on macOS 10.12, and Ubuntu 18.04. On Windows 10x64 the TypeError: window is undefined error is not displayed as well but there is still one error displayed (see screenshot). I don't know if this is expected or is tracked somewhere else. Should we close this as verified and open a new bug for the remaining error? Thank you!

Flags: needinfo?(jdescottes)

(In reply to Alexandru Trif, QA [:atrif] from comment #11)

Created attachment 9189803 [details]
error

Reproduced the issue with Firefox 83.0a1 (20201007155036) on Windows 10x64.
The issue is verified fixed with Firefox 84.0b4(20201122152513) on macOS 10.12, and Ubuntu 18.04. On Windows 10x64 the TypeError: window is undefined error is not displayed as well but there is still one error displayed (see screenshot). I don't know if this is expected or is tracked somewhere else. Should we close this as verified and open a new bug for the remaining error? Thank you!

Wrong screenshot, I think?

Flags: needinfo?(alexandru.trif)

The screenshot might be correct, Alexandru said the window is undefined error is gone, but now there is a new error Error reading typed URL history ... from https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/browser/components/migration/MSMigrationUtils.jsm#83

This other error is unrelated to DevTools, I would suggest opening a new bug in the Firefox/Migration component is this is reproducible on a clean profile https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Migration ?

Flags: needinfo?(jdescottes)

(In reply to Julian Descottes [:jdescottes] from comment #14)

The screenshot might be correct, Alexandru said the window is undefined error is gone,

Oops, yes, you're right, I misread. Sorry!

but now there is a new error Error reading typed URL history ... from https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/browser/components/migration/MSMigrationUtils.jsm#83

This other error is unrelated to DevTools, I would suggest opening a new bug in the Firefox/Migration component is this is reproducible on a clean profile https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Migration ?

Indeed. It's probably an issue specific to win10 machines that don't have those registry keys because they never had old old MSEdge installed (which used to use those keys).

(In reply to Julian Descottes [:jdescottes] from comment #14)

The screenshot might be correct, Alexandru said the window is undefined error is gone, but now there is a new error Error reading typed URL history ... from https://searchfox.org/mozilla-central/rev/0d6e8b21569f93a1e1ae8e377ab10f43a6cb12c1/browser/components/migration/MSMigrationUtils.jsm#83

This other error is unrelated to DevTools, I would suggest opening a new bug in the Firefox/Migration component is this is reproducible on a clean profile https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Migration ?

Yes, it happens every time with a new profile. Thank you for the responses and sorry for the late reply. I've opened bug 1680507. Closing this as verified based on the above.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(alexandru.trif)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: