267403 - reload and stop toolbar buttons don't update until an event is posted to the run queue

Status: VERIFIED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[Paul Sowden]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-gb; rv:1.8a5) Gecko/20041020 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-gb; rv:1.8a5) Gecko/20041020 Camino/0.8+

When a page is reloaded using the toolbar icon and the mouse is left hovering
the icons don't update (the stop and reload buttons don't swap state) if there
is no input.

Reproducible: Always
Steps to Reproduce:
1. Load up a page
2. Click the reload button with the mouse and then don't touch the mouse
3. Once the page is loaded the reload and stop buttons are not updated until the
mouse if moved.
Actual Results:  
The stop button remains active and the reload button inactive.

Expected Results:  
The stop button should become inactive and the reload button should become
active, allowing the user to again reload the page.

This is the same behaviour that was displayed in bug 231581

Comment 1

[Mina]

Confirming with 2004102808 (v0.8+), 10.3.5

Comment 2

[Jasper]

Confirmed.

Comment 3

[Mike Nolte]

Confirmed in 2004113008 (v0.8+), Mac OS X 10.3.6.

This bug is also reproduceable by using Command-R instead of clicking the reload
button.

This bug is also reproduceable by clicking on "Reload Page" in the "View" menu.

Comment 4

[Mina]

Still here in 2005032908 (v0.8+), Mac OS X 10.3.8

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

When the cursor is hovering anywhere over the toolbar (location field, search
field) when a page finishes loading, the stop button does not become inactive
and the reolad button doesn't become active.  It's any sort of page-load; it
doesn't have to be triggered by reload....

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #5)
> When the cursor is hovering anywhere over the toolbar (location field, search
> field) when a page finishes loading, the stop button does not become inactive
> and the reolad button doesn't become active.  It's any sort of page-load; it
> doesn't have to be triggered by reload....

This is not correct any more.  If the cursor is elsewhere on the page, the
buttons still don't update until the cursor moves.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** Bug 302592 has been marked as a duplicate of this bug. ***

Comment 8

[Paul Borokhov (lensovet)]

...boy this is annoying. time to vote.

Comment 9

[Chris Lawson (gone)]

Anyone (pink, Smokey, Simon, Wevah) have any idea why this might be happening?

cl

Comment 10

[Simon Fraser [no longer active]]

Yes; I think the toolbar only updates its items on mouse move. We're changing
state under the toolbar, so we probably need to call [NSToolbar
validateVisibleItems] or something.

Comment 11

[Samuel Sidler (old account; do not CC)]

This is definitely annoying, but as it stands, probably not 1.0 material.

Moving to 1.1.

Comment 12

[Stuart Morgan]

*** Bug 326951 has been marked as a duplicate of this bug. ***

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

From Hendrix:

Name:    Rick
Product: Camino
Summary: validateVisibleItems

Comments:
You want this guy called whenever you do something in your run loop
(your methods) that changes the status of your toolbar buttons.

If you don't, the user will not see the changes until something else
enters the run loop. Such as a mouse move. This is not good.

The call is simple and takes no arguments. And I know it says you
shouldn't invoke it, but that's just plain 101% wrong.

-(void)validateVisibleItems

Called on window updates with the purpose of validating each of the
visible items. You use this method by overriding it in a subclass; you
should not invoke this method. The toolbar calls this method to iterate
through the list of visible items, sending each a validate message.
Override it and call super if you want to know when this happens. 

Comment 14

[Nick Kreeger]

Taking

Comment 15

[Nick Kreeger]

Attachment: Proposed patch

Since the toolbar validation does not take place in |validateVisibleItems:| in BWC, this is the simplest fix.

Comment 16

[Nick Kreeger]

Attachment: Patch Deuce

After some further research, where we validate the refresh button, we check to see if the browser is busy, but that flag isn't set in BrowserWrapper until after the |setLoadingActive:| call is made.

This patch simply moves the |mIsBusy| boolean above the call to the browser window delegate with |setLoadingActive:|

Comment 17

[Chris Lawson (gone)]

Comment on attachment 227726 [details] [diff] [review]
Patch Deuce

r=me

Comment 18

[Nick Kreeger]

Updating summary.

Comment 19

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 227726 [details] [diff] [review]
Patch Deuce

sr=pink

Comment 20

[Nick Kreeger]

Fixed trunk and branch.

Comment 21

[Chris Lawson (gone)]

V. fixed with latest trunk nightly.