517772 - In Gmail, "More Actions" Drop Down Menu Appears Momentarily Under Left-Most Button

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[WildcatRay]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090920 Namoroka/3.6a2pre Firefox/3.5.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090920 Namoroka/3.6a2pre Firefox/3.5.3

Gmail "More Actions" drop down menu appears momentarily under left-most button when first viewing message listings. It appears this happens when first clicking on "More Actions" button for any item on left-hand side (Inbox, Starred, Sent Mail, etc.).

Subsequent tries do not yield this bug. You must go to another item on the left hand side like Starred, Sent Mail, etc., including any user-defined labels. Each time you go to one of these and then click on the “More Actions” button, the drop down will first appear under the left-most button before shifting to where it should be.

I am seeing this in Minefield (with only the Java Quick Starter add-on) and Namoroka Branch, but not Fx 3.5.3.


Reproducible: Always

Steps to Reproduce:
1. Log in to Gmail. Should be in Inbox.
2. Click on the "More Actions" button.

Subsequent tries without going to another of the left side items first will result in the drop down menu appearing where it should.

Actual Results:  
The "More Actions" drop down menu appears momentarily under the left-most button ("Archive" in Inbox, "Remove Label "Name"" in a labeled message listing) before it shifts over to under the "More Actions" button.

Expected Results:  
The drop down menu should always appear underneath when clicking the "More Actions" button.

Comment 1

[WildcatRay]

Need help IDing the regression window. I first noticed this at the end of July, 2009.

Comment 2

[Peter van der Woude [:Peter6]]

this regressed between the 20090721 and 20090722 nightlies
maybe Bug 505186 ?

Comment 3

[Martijn Wargers (dead)]

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-07-21+04%3A00%3A00+&enddate=2009-07-22+06%3A00%3A00
This was when bug 352093 related check-ins were checked in.

Comment 4

[Martijn Wargers (dead)]

Hmm, is this the right regression range? With a 2009-07-20 build, I can see this bug with the "Move To", "Labels" and the "More actions" menu when I'm reading one of the mails.

Comment 5

[WildcatRay]

(In reply to comment #4)
> Hmm, is this the right regression range? With a 2009-07-20 build, I can see
> this bug with the "Move To", "Labels" and the "More actions" menu when I'm
> reading one of the mails.

I was able to see this bug when first having logged in to Gmail as early as the 20090716/20090717 Trunk nightly builds. However, unlike later builds like the range Peter IDed, I only saw it once unless I logged out and logged back in. Quite frustrating.

Comment 6

[WildcatRay]

(In reply to comment #5)
> (In reply to comment #4)
> > Hmm, is this the right regression range? With a 2009-07-20 build, I can see
> > this bug with the "Move To", "Labels" and the "More actions" menu when I'm
> > reading one of the mails.
> 
> I was able to see this bug when first having logged in to Gmail as early as the
> 20090716/20090717 Trunk nightly builds. However, unlike later builds like the
> range Peter IDed, I only saw it once unless I logged out and logged back in.
> Quite frustrating.

I just tested the 2009070104 Trunk nightly. The very first time after logging I saw the drop down menu first appear under the Archive button before shifting over to where it was supposed to be. Subsequent attempts failed to show this bug.

Comment 7

[Martijn Wargers (dead)]

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-05-05+04%3A00%3A00&enddate=2009-05-07+07%3A00%3A00
Probably regression from bug 67752.

Comment 8

[WildcatRay]

(In reply to comment #7)
> http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-05-05+04%3A00%3A00&enddate=2009-05-07+07%3A00%3A00
> Probably regression from bug 67752.

I have just tested the 20090620 and 20090615 (available at http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2009/06/2009-06-15-04-mozilla-central/) when is saw your comment. I saw the bug on the 20090620 build, but not on the 20090615 build. Can you confirm whether or not you see the bug on the 20090615 build, please? Thanks.

Comment 9

[Martijn Wargers (dead)]

No, I get the regression range I mentioned in comment 7.

Comment 10

[WildcatRay]

(In reply to comment #9)
> No, I get the regression range I mentioned in comment 7.

Thanks.

Comment 11

[Marcia Knous [:marcia]]

Wondering if this bug I filed back in July is a similar issue: Bug 507747

Comment 12

[Boris Zbarsky [:bzbarsky]]

Seems possible....  Is it reliably reproducible?  If so, does the problem go away if you run with GECKO_REFLOW_INTERRUPT_MODE=counter GECKO_REFLOW_INTERRUPT_FREQUENCY=1000000 GECKO_REFLOW_INTERRUPT_CHECKS_TO_SKIP=1000000 in your environment?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ok, I just tried comment 12 (which disables reflow interruption).  I still see this problem.  So not an ireflow issue...

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Mats, you could look at this...

Comment 15

[Mats Palmgren (inactive)]

Sure.  I can reproduce this bug on Linux and OSX, but only once per left-side
item, then I need to reload for it to appear again.  I can reproduce bug 507747
on OSX, but not on Linux so far.

Comment 16

[Mats Palmgren (inactive)]

It looks like an ireflow regression after all.  I cloned rev 47e935bd7f3b,
which is the rev before ireflow landed, and that works fine.  Then applied changeset a08d1947ec5a and with that I can reproduce the problem.

Regarding the settings in comment 12: I tried those on trunk and get the
same result as Boris - the bug still occurs.  However, in the tree with
the initial ireflow landing it does make the problem go away, both on Linux
and OSX. (fwiw, bug 507747 does not occur in that tree, so it might depend
on an additional later change).

So far I've gathered that the "dropdown menu" is an abs.pos. DIV.
It looks like it's the corresponding frame that is interrupted.

Comment 17

[Mats Palmgren (inactive)]

AFAICT so far, this isn't our bug.  Looking at the stack traces
of Reflow and Paint of the abs.pos. dropdown menu, what the Gmail
script does when clicking on a button like "Move To" is something
like this: in the ButtonPress handler the frame for the dropdown is
created, then there is a GetBoundingClientRect() call on the "Move To"
element, causing a Flush_Layout, so we process pending reflows, including
the dropdown menu.  I can see that in this first reflow it has:
style="-moz-user-select: none;" then on subsequent reflows it has
style="-moz-user-select: none; left: 199px; top: 28px;" so I'm guessing
this is what they used GetBoundingClientRect() for -- to align the dropdown
with the button.  The problem occurs if reflow is interrupted, then there
can sneak in an Expose event which paints the frame in the position
it got with unspecified left/top.

I'll debug some more to see if I can gather some more details but I'm
leaning towards INVALID, and that the bug is in Gmail (probably an 
easy fix for them).

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If they do a getBoundingClientRect call, then change the dropdown's 'style' attribute, all in the same script execution, we shouldn't paint it at the old position if we can avoid it; that's our bug.

Comment 19

[Mats Palmgren (inactive)]

Ok, here's some more details below... the problem appears to be that
there's a pending reflow event which suppresses interruptible reflow
from PresShell::WillPaint:

OnButtonPressEvent
  nsIDOMCSS2Properties_GetPosition ("Move to")
    FlushPendingNotifications (Flush_Style)
      RecreateFramesForContent
        ContentInserted (style="-moz-user-select: none;")
          creates placeholder and abs.pos. frames for it...
  nsIDOMNSElement_GetBoundingClientRect ("Move to")
    FlushPendingNotifications (Flush_Layout)
      ProcessReflowCommands
        ReflowAbsoluteFrame (style="-moz-user-select: none;") rect.x=180
        ... reflow is interrupted ...
  SetAttr style='-moz-user-select: none; left: 199px; top: 28px;'
----

OnExposeEvent
  PresShell::WillPaint
    we have a pending reflow event and mSuppressInterruptibleReflows=true
    so FlushPendingNotifications(Flush_InterruptibleLayout) will not
    call ProcessReflowCommands
  PresShell::Paint
    MarkFramesForDisplayList (style="-moz-user-select: none; left: 199px; top: 28px;"  rect.x=180) frame is DIRTY!!!
    nsDisplayList::Paint     .... same ....
      ... paints it in the wrong position
----

PresShell::ReflowEvent::Run
    reflows our frame with "left: 199px; top: 28px;"
----

OnExposeEvent
  ... paints it in the correct position

Comment 20

[Mats Palmgren (inactive)]

Attachment: wip1

This seems to fix it in the tree I cloned from the initial ireflow landing.
It doesn't fix it in a trunk build though...

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #19)
> OnButtonPressEvent
>   nsIDOMCSS2Properties_GetPosition ("Move to")
>     FlushPendingNotifications (Flush_Style)
>       RecreateFramesForContent
>         ContentInserted (style="-moz-user-select: none;")
>           creates placeholder and abs.pos. frames for it...
>   nsIDOMNSElement_GetBoundingClientRect ("Move to")
>     FlushPendingNotifications (Flush_Layout)
>       ProcessReflowCommands
>         ReflowAbsoluteFrame (style="-moz-user-select: none;") rect.x=180
>         ... reflow is interrupted ...

Why is this reflow interrupted? Is it a pending button release event? Did we correctly let the reflow run for the minimum reflow time before allowing the interrupt? Actually, how can this reflow be interrupted, it should be non-interruptible since we need layout to be flushed!

Comment 22

[Boris Zbarsky [:bzbarsky]]

So...  see, the settings in comment 12 effectively disable all reflow interruption.  I just tried a trunk debug build with those settings and breakpoints set in all the places where we might interrupt reflow.  I see the gmail bug, and none of the breakpoints are hit.

Next order of business: determine when the bug started showing up even with those env vars.  It might just be that there's an ireflow issue _and_ something else going on here.

Comment 23

[Boris Zbarsky [:bzbarsky]]

OK, that regression happened between the 2009-07-21-03 (rev f2a58ffcd00c) build and the 2009-07-22-03 (rev 02f8bf10f441) build.  Pushlog:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f2a58ffcd00c&tochange=02f8bf10f441

Something to do with the subframe widget removal?

Comment 24

[Mats Palmgren (inactive)]

I concur with Boris, it appears to be two separate issues here.
I get the same regression range as above for the 2nd issue.  I cloned
rev 5b0d9f36c0b3 and can reproduce the bug also with the env vars set.
I then reversed bug 352093 part 14 only (as Boris suggested), which
fixed the 2nd issue, ie I can only reproduce the bug with ireflow enabled.
Applying wip1 onto that fixes the bug.  As a sanity check, backing out
part 14 and applying wip1 on trunk fixes the bug too.

Comment 25

[Mats Palmgren (inactive)]

Attachment: wip2

It think the regression that occurred with the widget removal changes
is that the Expose event now targets an enclosing widget and it seems
WillPaint isn't called for any sub-shells that we build DisplayLists
for.  I can see that there is a restyle request queued for the abs.pos.
at hand, but in an embedded shell.
FWIW, this patch appears to make the problem slightly harder to reproduce
but still does not fix it completely...

Comment 26

[Mats Palmgren (inactive)]

IsSafeToFlush is what blocked it... which seems natural given my hack to
nsDisplayListBuilder::EnterPresShell  (we shouldn't call WillPaint()
from there of course since it could destroy the world), so in some
cases I just revoked the event without actually flushing.
So, I'll try to make WillPaint itself recurse into sub-shells instead...

Comment 27

[Boris Zbarsky [:bzbarsky]]

Wouldn't it make more sense to make the WillPaint call in nsViewManager::DispatchEvent look more like the one in nsViewManager::FlushPendingInvalidates?  It's not really enough to WillPaint on sub-shells; you really want to get sibling shells too, etc...  Note that there's an XXX comment about this in DispatchEvent.

Comment 28

[Mats Palmgren (inactive)]

Attachment: wip3

FWIW, WillPaint on sub-shells appears to fix the problems with Gmail.

(In reply to comment #27)
> Wouldn't it make more sense to make the WillPaint call in
> nsViewManager::DispatchEvent look more like the one in
> nsViewManager::FlushPendingInvalidates?

Ok, I'll look into that...

Comment 29

[Mats Palmgren (inactive)]

Attachment: Patch rev. 4

It doesn't seem necessary to process a pending reflow event early in
PresShell::WillPaint to fix the problems with Gmail, so I'm leaving
it as is.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 406587 [details] [diff] [review]
Patch rev. 4

Maybe call the new method CallWillPaintOnObservers?

roc, can you take a look at this too?

Comment 31

[Mats Palmgren (inactive)]

CallWillPaintOnObservers it is.
http://hg.mozilla.org/mozilla-central/rev/66c1213056bd

Comment 32

[WildcatRay]

Confirming the menu does appear where it should when first accessed. Thanks.

Comment 33

[Mats Palmgren (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/17c96511214d

Comment 34

[WildcatRay]

I just noticed this bug, again, in Trunk (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091113 Minefield/3.7a1pre Firefox/3.5.5 - Build ID: 20091113043249) I need to research this further for regression range.

Comment 35

[WildcatRay]

This regressed in the 20091110 Trunk nightly. (The 20091109 nightly did not show this.)

Changeset: http://hg.mozilla.org/mozilla-central/rev/f4ef40336cc6

What I noted is that the bug presents itself the first time the More Actions button is clicked on Inbox or any user label. Afterward, whether switching back and forth between the Inbox or a label, if the More Actions button has been clicked once when in the Inbox or a label, the bug no longer appears in the Inbox or a label. To get the bug to appear, again, I had to sign out or close and then restart the browser. Evidently, there is something about going to a label the first time in a session--and similarly the Inbox when first starting Gmail--that is causing this bug to manifest itself.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Sounds like a likely regression from bug 498340.  See bug 527864.  Might be worth filing as a separate bug...

Comment 37

[WildcatRay]

This appears to be fixed, again, in recent builds. I'd say it is no longer an issue.