1425109 - New (Ping Pong) Loading animation not showing for iframe changes

Status: RESOLVED WONTFIX

(Firefox :: Tabbed Browser, defect, P3)

Description

[Jason]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.84 Safari/537.36

Steps to reproduce:

Go to https://codepen.io/Onomicon/pen/ZbQmgv
(just an example, any site with iframe and code that changes the iframe src have this problem)
Click on the links that changes the iframe src.


Actual results:

The new Ping Pong loading animation on the tab doesn't show.  Only the Reload button turns to X, and the floating loading status bar will show during the load.


Expected results:

The old spinning circle animation would show when iframe is being loaded.

Due to this change, it feels like the action to change the iframe src isn't happening, causing the user to click multiple times on the action.

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
20171213100213

Comment 2

[Timothy Nikkel (:tnikkel)]

I doubt this is layout. Likely either dom, document navigation, or in the front end.

Comment 3

[marc.j.hassan]

Attachment: Remove isTopLevel check for iframe throbber

Comment 4

[marc.j.hassan]

See attached proposed commit that removes the isTopLevel check for the throbber.

However, I wonder if this bug was actually an intentional change in 57.

Comment 5

[Jason]

If it is intentional then at least make it configurable in about:config.
Tons of sites where a menu changes stuff in iframe.
In the flip side, I could see a site where iframe is in constant loading of something in the background which will throw off the loading indicator's purpose.

Comment 6

[Gingerbread Man]

(In reply to marc.j.hassan from comment #4)
> See attached proposed commit

Thank you for the patch. Since there's disagreement on the proper component for this, I'm not sure who to flag for review. NI :bz

Comment 7

[Boris Zbarsky [:bzbarsky]]

When in doubt, flag the person who wrote or reviewed the code being changed.

In this case that would be Jared or Gijs, per bug 1380150.  Looks to me like the attached patch would simply regress that bug; I'll let Jared decide how he wants to handle this.  ;)

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

This was done by request from UX. The reload button still changes to the stop button but UX is concerned that the ping-pong animation is too distracting. Eric, now that we have lived with this change for a while, are you still confident about it?

Comment 9

[Jason]

Wow, didn't think my little report would involve so many people.
I'll give my 2 cents.
I'm no UX, but without any animation, it just feels like Firefox isn't doing anything, since everything looks static.  Doesn't convey the feeling of Firefox is "doing work".

Comment 10

[marc.j.hassan]

I found this ticket because it was something that I was missing in 57. Particularly with pages that contain large iframes, where the user may not even know they are interacting with an iframe, the user isn't sure if their interaction did anything until suddenly the page flashes. The reload button changing to stop is not noticeable to me. I am also not a UX person.

Side note: the behavior in 57 is like the behavior in Google Chrome, where there is no indication of page loads within an iframe (not even the reload button changes).

Comment 11

[Eric Pang [:epang] UX]

I agree that the loading indicator missing for the iframe content feels wrong.
Is it possible for the iframe to trigger the loading indicator the same way it would if it was in it’s own tab?

I feel this will be the right balance and keeps bug 1380150 for regressing, which fixed important UI problems and increased perceived performance.

Comment 12

[marc.j.hassan]

Eric- Can you explain what you mean by this?:

> Is it possible for the iframe to trigger the loading indicator the same way
> it would if it was in it’s own tab?

Comment 13

[Eric Pang [:epang] UX]

(In reply to marc.j.hassan from comment #12)
> Eric- Can you explain what you mean by this?:
> 
> > Is it possible for the iframe to trigger the loading indicator the same way
> > it would if it was in it’s own tab?

Hi Marc, 

I'm hoping there's a way to treat the contents within the iframe as part of the page loading.
For example, on first load the indicator should take into account the page content inside and outside of the iframe.
After first load any actions in the iframe are treated as they would be outside of the iframe. 

Let me know if that makes more sense!

Comment 14

[marc.j.hassan]

With the patch I have attached, I am seeing this:

If the iframe is part of the page source:

1) On initial page load, load indicator spins until the page has loaded (including iframe). Load indicator stops.
2) On navigation within iframe, load indicator starts. Once iframe is done loading, load indicator stops.

I believe that is the desired behavior you are describing.

If the iframe is added dynamically after page load:

1) On initial page load, load indicator spins until the page has loaded (excluding the non-existent iframe). Load indicator stops.
2) When iframe is added and starts loading, load indicator starts. Once iframe is done loading, load indicator stops.
3) On navigation within iframe, load indicator starts. Once iframe is done loading, load indicator stops.

Comment 15

[Eric Pang [:epang] UX]

(In reply to marc.j.hassan from comment #14)
> With the patch I have attached, I am seeing this:
> 
> If the iframe is part of the page source:
> 
> 1) On initial page load, load indicator spins until the page has loaded
> (including iframe). Load indicator stops.
> 2) On navigation within iframe, load indicator starts. Once iframe is done
> loading, load indicator stops.
> 
> I believe that is the desired behavior you are describing.
> 
> If the iframe is added dynamically after page load:
> 
> 1) On initial page load, load indicator spins until the page has loaded
> (excluding the non-existent iframe). Load indicator stops.
> 2) When iframe is added and starts loading, load indicator starts. Once
> iframe is done loading, load indicator stops.
> 3) On navigation within iframe, load indicator starts. Once iframe is done
> loading, load indicator stops.

Hi Marc, the behavior you describe sounds right.  Are you able to push to try so I can test it out?

Comment 16

[marc.j.hassan]

I do not have access to push to try. Here is my Commit Access request: https://bugzilla.mozilla.org/show_bug.cgi?id=1430618

Comment 17

[Eric Pang [:epang] UX]

(In reply to marc.j.hassan from comment #16)
> I do not have access to push to try. Here is my Commit Access request:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1430618

@jwein are you able to give Marc access to push to try to I can test out his patch?

Comment 18

[marc.j.hassan]

Can someone either vouch for me, or push to try for me? I am a new developer. Here is my Commit Access request: https://bugzilla.mozilla.org/show_bug.cgi?id=1430618

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

I have vouched for Marc on the commit access request bug.

Comment 20

[marc.j.hassan]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e43b9ba07e5feba15c44878cd38affde7fcc756

Comment 21

[Eric Pang [:epang] UX]

(In reply to marc.j.hassan from comment #20)
> try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=2e43b9ba07e5feba15c44878cd38affde7fcc756

Marc, can you make a build for OSX?  Sorry I should have specified that before! Thanks!

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

I added a new job for OSX opt build to that try push. It should be ready in an hour or so.

Comment 23

[Eric Pang [:epang] UX]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #22)
> I added a new job for OSX opt build to that try push. It should be ready in
> an hour or so.

Thanks Jared for the try push.

The flickering that can occur between the loading indicator, burst and favicon causes the experience to feel unpolished and unrefined. This also impacts perceived performance since secondary loads make the page feel much more sluggish when most of it is usually irrelevant to the user.  Content in an iframe is a good reason to show the loading indicator but if it can't be done in a way that doesn't revert the fixes of bug 1380150 I rather leave the experience as is.  Also, the user still has the refresh icon and progress indicator in the bottom left of the browser as indicators.

> If it is intentional then at least make it configurable in about:config.

I'm all for the options to make this configurable in about:config

Comment 24

[Jason]

How about making the X on the refresh icon to be animated?  Perhaps a circle line that spins around the X.
The concern is lack of "movement" on the entire browser while the iframe content is being loaded.

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

The animation on the Reload->Stop after a page has finished loaded was disabled because users said it was too distracting. 

I am against adding a preference to change this behavior because we will then need to maintain and test another code path and we have limited resources.