Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usability issues with vomnibar in iframe (post-1.46 only) #1361

Closed
smblott-github opened this issue Dec 19, 2014 · 8 comments
Closed

Usability issues with vomnibar in iframe (post-1.46 only) #1361

smblott-github opened this issue Dec 19, 2014 · 8 comments

Comments

@smblott-github
Copy link
Collaborator

In post-1.46, the vomnibar is in an iframe (see #1214 and #1139). Using this for everyday browsing, I've found:

  • The favicon and refresh button twitch every time the vomnibar is opened.
  • The vomnibar is slower to load. Not much slower, but noticeably slower.
  • Several times, by the time the vomnibar opens, I've already typed the first few characters of the query, so the vomnibar misses them. (Was that possible previously?)

Any ideas as to how this might be improved, @mrmr1993?

Probably not related, but I'll mention anyway:

  • Several times, a bookmark I expected to open in a new foreground tab, opened instead in a new background tab. (However, I could just have fumbled the keyboard somehow.)

(This does not affect master or 1.49.)

@mrmr1993
Copy link
Contributor

Easy fix:

  • preload the Vomnibar iframe
  • use chrome.storage.local to communicate the options (rather than URL encoding) so that the iframe location isn't changed each time the Vomnibar is opened.

@smblott-github
Copy link
Collaborator Author

Several times, a bookmark I expected to open in a new foreground tab, opened instead in a new background tab. (However, I could just have fumbled the keyboard somehow.)

This just happened again. Definitely no keyboard fumble this time. (Could still be unrelated to vomnibar in iframe.)

@smblott-github
Copy link
Collaborator Author

preload the Vomnibar iframe

I just did a side-by-side comparison on Google Plus after scrolling down for two reloads. There's a significant difference in the vomnibar load speed between master and post-.146. We need to either fix this or revert.

@mrmr1993
Copy link
Contributor

I'm concerned about the weight of preloading the vomnibar iframe in every frame, especially when we have lot (eg. Google inbox). Since the vomnibar only acts on the whole tab, it makes sense to only preload it for the top frame in nearly all cases.

For top-level framesets, however, we can't inject the iframe. At the moment it's hard to communicate between frames, and so difficult to delegate the vomnibar to one or more of its children, and we might get into an awkward situation with nested framesets.

My current idea for resolving this is:

  • each frame calculates it's position in the frame hierarchy by traversing parent.frames
  • all frames inform the background page of their document.body.tagName and their position in the hierarchy
  • the background page tells all non-frameset frames that are highest in their branch of the frame tree to load the Vomnibar
  • a frame requesting the vomnibar shows the vomnibar in the outermost frame in its branch that can show it

This seems really inelegant, especially for an edge-case for a depreciated tag. Do you have any ideas for how to improve the situation @smblot-github?

@smblott-github
Copy link
Collaborator Author

My current idea for resolving this is: ...

Interesting idea, but quite complicated. I'd say take it one step at a time.

Suggest:

  • Preload vomnibar in omni mode always (i.e. no need to pass arguments). Verify that the load-delay does indeed go away, and that there are no unforeseen gotchas. Then, if that looks good...
  • Use chrome.storage to pass arguments to vomnibar. Verify that there are no gotchas here either.

Only then would I worry about optimisation. The common case is just one frame, or perhaps a couple. There may be nothing to worry about in these cases. In other cases (like inbox and gmail), we may get away with just not preloading the vomnibar in frames which are too small to show it anyway (e.g., frames need a certain width and height for it to be usable at all - you can't use the vomnibar in a 10x10 frame, and it may not be that useful in a 10x100 frame, or 100x10).

@mrmr1993
Copy link
Contributor

  • Use chrome.storage to pass arguments to vomnibar. Verify that there are no gotchas here either.

Now I'm leaning more towards using window.message on the iframe and passing a port to the vomnibar. I'll have a look at the security implications for this and check that we're not making ourselves vulnerable, but it's otherwise a much more elegant solution.

@mrmr1993
Copy link
Contributor

I've pushed PR #1382 to fix this.

@smblott-github
Copy link
Collaborator Author

Closing.
Addressed in #1390.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants