Page MenuHomePhabricator

Page action overlay has displaced corner on initial load
Closed, ResolvedPublic

Description

The corner that is presumably supposed to connect to the watch star, is actually placed too far out connecting to nothing.

This happens 9 out of 10 page loads in latest Chrome on iPad (iOS 8.3), and also reproducible on desktop.

At http://en.m.wikipedia.beta.wmflabs.org/wiki/Alcatraz_Island, logged-out, after opting into Beta from the settings.


While not possible on mobile, when letting the page fully load and doing a window resize, the corner usually corrects itself. I guess the styles are computed prematurely and hitting a race condition.

Perhaps the distance can be computed relatively from the top right corner of the overlay (using the known width of the icon) instead of absolutely for from top left corner of the page container (which depends on a lot of variables only known at run time).

Event Timeline

Krinkle created this task.Jun 20 2015, 4:49 AM
Krinkle raised the priority of this task from to Needs Triage.
Krinkle updated the task description. (Show Details)
Krinkle added projects: Gather, MobileFrontend.
Krinkle added a subscriber: Krinkle.
Restricted Application added a project: Readers-Web-Backlog. · View Herald TranscriptJun 20 2015, 4:49 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
rmoen moved this task from To Do to Doing on the Reading-Web-Sprint-50-The-X-Files board.

Change 220013 had a related patch set uploaded (by Robmoen):
Fix pointer position in page action overlay

https://gerrit.wikimedia.org/r/220013

@Jdlrobson What are your thoughts about accepting this as a regression fix with plans to investigate the root cause? See my response please.

I'm just concerned that we keep hitting problems with this thing, and as annoying as they are I'd rather we got to the bottom of it. Since it's a visual bug it's hard to catch this re-occurring with tests.

Let me have a poke around at the code and see if I can work out what's happening here.

Change 220013 had a related patch set uploaded (by Jdlrobson):
Fix pointer position in page action overlay

https://gerrit.wikimedia.org/r/220013

There was a bug in the banner image. It was deleting itself but not triggering a refresh event.

Change 220979 had a related patch set uploaded (by Jdlrobson):
Removing banner should trigger a redraw

https://gerrit.wikimedia.org/r/220979

The root cause is explained here: https://gerrit.wikimedia.org/r/#/c/220980/
The banner patch seems to solve the issue only because it runs late.

Change 220979 merged by jenkins-bot:
Removing banner should trigger a redraw

https://gerrit.wikimedia.org/r/220979

Change 221002 had a related patch set uploaded (by Robmoen):
Delay tutorial so it is more noticeable to the user

https://gerrit.wikimedia.org/r/221002

Change 220013 abandoned by Robmoen:
Fix pointer position in page action overlay

Reason:
Abandoned in favor of I37e3a548884ebfb8934ba80544400ad387fbf839

https://gerrit.wikimedia.org/r/220013

Change 221002 merged by jenkins-bot:
Delay tutorial so it is more noticeable to the user

https://gerrit.wikimedia.org/r/221002

phuedx added a subscriber: phuedx.

Verified on BC. I've raised a bug for the spurious behaviour of the lead image when there's no image to display (T103967).

phuedx closed this task as Resolved.Jun 26 2015, 9:41 AM
phuedx moved this task from Ready for Signoff to Done on the Reading-Web-Sprint-50-The-X-Files board.
bmansurov reopened this task as Open.EditedJun 26 2015, 10:11 AM

Imagine we disable the banner images module and don't delay showing the tutorial, what will you see? The same bug!

@Jdlrobson, so please instead of dismissing my patch and saying that you don't understand it, let me know which part is confusing and I'll address it in a subsequent patch.

Also, why did we start delay showing the overlay all of a sudden? To "fix" the bug? Or was there a design decision?

Imagine we disable the banner images module and don't delay showing the tutorial, what will you see? The same bug!

This sounds like a different variant of the bug that you should raise a task that should be evaluated. Currently we have a banner images module. We do delay showing it.

@Jdlrobson, so please instead of dismissing my patch and saying that you don't understand it, let me know which part is confusing and I'll address it in a subsequent patch.

The bit thats been confusing me is not having a bug with reproducible steps as I've said on the patch. I paired with @rmoen yesterday and we got a fixed for the issue that @Krinkle raised and that @phuedx has now signed off on.

On subject of timeout we decided that knowing that the banner will reposition itself and that the user doesn't need to have to see it instantly it was better to delay showing the pointer until the design had stabilised so it doesn't move.

I think if we fix asynchronous loading of the banner as in the bug Sam opened we could always reconsider this decision.

phuedx closed this task as Resolved.Jun 29 2015, 12:24 PM

With @bmansurov's blessing, I'm closing this as Resolved. Thanks for the discussion y'all.

That's not to imply that the discussion stops here…