Skip to content
This repository has been archived by the owner. It is now read-only.

/ fxa-content-server Archived

Loading of settings page via Manage link in Sync preferences page is not smooth #3053

Closed
ckarlof opened this issue Sep 11, 2015 · 9 comments
Closed

Loading of settings page via Manage link in Sync preferences page is not smooth #3053

ckarlof opened this issue Sep 11, 2015 · 9 comments
Assignees
@shane-tomlinson
Milestone
FxA-0: quality

Comments

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Sep 11, 2015

I get some flashing and see the Firefox logo briefly.
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 15, 2015

cc @zaach
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 15, 2015

ref #3007 as one possible source of slowness.
@ckarlof
Copy link
Contributor Author

@ckarlof ckarlof commented Sep 15, 2015

Also, /settings seems to take a while to load, even after it's cached (I get the spinner all the time).
@zaach
Copy link
Contributor

@zaach zaach commented Sep 15, 2015

Also, /settings seems to take a while to load, even after it's cached (I get the spinner all the time).

Hints at #3007 and other API calls.
@ckarlof ckarlof mentioned this issue Sep 18, 2015
Closed
@rfk
Copy link
Member

@rfk rfk commented Sep 21, 2015

Here's the series of XHR I see when reloading /settings (to try to use a warm cache):

  • GET /session/status
  • POST /certificate/sign
  • POST /authorization
  • GET /profile
  • GET /session/status
  • GET /session/status
  • GET /session/status
  • GET /session/status
  • GET /session/status
  • GET /profile
  • POST /certificate/sign
  • POST /authorization
  • GET /lookup-user
  • POST /destroy
  • GET /avatar

There's also likely some non-trivial CPU being burnt when generating those two signed assertions.
@rfk rfk added this to the FxA-0: quality milestone Sep 21, 2015
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 21, 2015

There's also likely some non-trivial CPU being burnt when generating those two signed assertions.

Huh, this was fixed once before: #2921
shane-tomlinson pushed a commit that referenced this issue Sep 21, 2015
Shane Tomlinson
fix(client): Only one sessionStatus check call should be made from /s…
Loading status checks…
d91fb4d 
…ettings

Remove the SettingsMixin from all the Settings pages except for the root
Settings view. This ensures only `/settings` calls isUserAuthorized.

Add tests for the settings-mixin

fixes #3053
@shane-tomlinson shane-tomlinson mentioned this issue Sep 21, 2015
Merged
@shane-tomlinson shane-tomlinson self-assigned this Sep 21, 2015
@shane-tomlinson shane-tomlinson mentioned this issue Sep 21, 2015
Closed
shane-tomlinson pushed a commit that referenced this issue Sep 22, 2015
Shane Tomlinson
fix(client): Only allow one profile request per account per tab session.
Loading status checks…
3baae12 
This avoids multiple XHR requests being made by multiple views when displaying
profile information at the same time.

issue #3053
@shane-tomlinson shane-tomlinson mentioned this issue Sep 22, 2015
Merged
shane-tomlinson pushed a commit that referenced this issue Sep 22, 2015
Shane Tomlinson
fix(client): Prevent the Firefox logo from being displayed on setting…
Loading status checks…
5548325 
…s pages.

The `settings` class ws added in beforeRender, which is only called after
an XHR request is made to determine the user's authentication status. On slow
connections this allows the Firefox logo and the white screen used
elsewhere in the sight to be displayed while the user's status is
being determined. Add the `settings` class early. If the user is not
authenticated, `beforeDestroy` will take care of removing the class.

* Add the `settings` class to the body in an overridden `render` function,
  before any XHR requests are made.

issue #3053
shane-tomlinson pushed a commit that referenced this issue Sep 22, 2015
Shane Tomlinson
fix(client): Prevent the Firefox logo from being displayed on setting…
Loading status checks…
f7ac43d 
…s pages.

The `settings` class ws added in beforeRender, which is only called after
an XHR request is made to determine the user's authentication status. On slow
connections this allows the Firefox logo and the white screen used
elsewhere in the sight to be displayed while the user's status is
being determined. Add the `settings` class early. If the user is not
authenticated, `beforeDestroy` will take care of removing the class.

* Add the `settings` class to the body in an overridden `render` function,
  before any XHR requests are made.

issue #3053
@shane-tomlinson shane-tomlinson mentioned this issue Sep 22, 2015
Merged
shane-tomlinson pushed a commit that referenced this issue Sep 22, 2015
Shane Tomlinson
fix(client): Prevent the Firefox logo from being displayed on setting…
Loading status checks…
596f840 
…s pages.

The `settings` class ws added in beforeRender, which is only called after
an XHR request is made to determine the user's authentication status. On slow
connections this allows the Firefox logo and the white screen used
elsewhere in the sight to be displayed while the user's status is
being determined. Add the `settings` class early. If the user is not
authenticated, `beforeDestroy` will take care of removing the class.

* Add the `settings` class to the body in an overridden `render` function,
  before any XHR requests are made.

issue #3053
@shane-tomlinson shane-tomlinson mentioned this issue Sep 22, 2015
Closed
shane-tomlinson pushed a commit that referenced this issue Sep 23, 2015
Shane Tomlinson
fix(client): Smooth out the /settings page load.
Loading status checks…
4a605e5 
Add `layoutClassName` capabilities to views.

If a view defines `layoutClassName`, the class name is added to the
`body` element as soon as rendering starts. The class name is
removed in destroy.

In router.js, help smooth out transitions by removing the previous
view only once the new view is ready to insert into the DOM. We previously
removed the old view before the new view's render call was made. If the
new screen's render function made XHR requests, this could leave a blank
screen.

issue #3053
shane-tomlinson pushed a commit that referenced this issue Sep 23, 2015
Shane Tomlinson
fix(client): Smooth out the /settings page load.
Loading status checks…
5013188 
Add `layoutClassName` capabilities to views.

If a view defines `layoutClassName`, the class name is added to the
`body` element as soon as rendering starts. The class name is
removed in destroy.

issue #3053
shane-tomlinson pushed a commit that referenced this issue Sep 23, 2015
Shane Tomlinson
fix(client): Smooth out the /settings page load.
Loading status checks…
37d775c 
Add `layoutClassName` capabilities to views.

If a view defines `layoutClassName`, the class name is added to the
`body` element as soon as rendering starts. The class name is
removed in destroy.

issue #3053
shane-tomlinson pushed a commit that referenced this issue Sep 23, 2015
Shane Tomlinson
fix(client): Smooth out the /settings page load.
Loading status checks…
c27ad5b 
Add `layoutClassName` capabilities to views.

If a view defines `layoutClassName`, the class name is added to the
`body` element as soon as rendering starts. The class name is
removed in destroy.

issue #3053
shane-tomlinson pushed a commit that referenced this issue Sep 23, 2015
Shane Tomlinson
fix(client): Smooth out the /settings page load.
Loading status checks…
207dfd6 
Add `layoutClassName` capabilities to views.

If a view defines `layoutClassName`, the class name is added to the
`body` element as soon as rendering starts. The class name is
removed in destroy.

issue #3053
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 5, 2015

for next 2 weeks / to check with other patches
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Oct 5, 2015

ref #3092, @shane-tomlinson to double check if this is "smooth enough"
@shane-tomlinson shane-tomlinson mentioned this issue Oct 9, 2015
Closed
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Oct 9, 2015

We only generate one certificate and make one session/status request now. We still create 2 OAuth tokens and make 2 profile requests. The first can't be helped, the 2nd we are working on.

The settings page is much smoother now. Manual tests tried:

  • Sign in from /signin
  • force refresh from /settings
  • refresh using cache from /settings
  • refresh and force refresh sub-panels

Closing as "Good enough for now."
shane-tomlinson pushed a commit that referenced this issue Oct 14, 2015
Shane Tomlinson
fix(client): Only allow one profile request per account per tab session.
Loading status checks…
c7067cd 
This avoids multiple XHR requests being made by multiple views when displaying
profile information at the same time.

issue #3053
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@shane-tomlinson shane-tomlinson

Labels
None yet
Projects
None yet
Milestone
FxA-0: quality
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
@zaach @rfk @vladikoff @shane-tomlinson @ckarlof @edwindotcom