From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; N; Win95; en-US) Mozilla/m13 BuildID: 2000022820 when I click a URL in Communicator, I get a nice "watch" cursor (the hourglass) so I know that I clicked it. in IE and now Mozilla, nothing changes cursor-wise when I click a link. it's confusing because it's hard to tell whether or not it's actually doing anything. Reproducible: Always Steps to Reproduce: 1. click a link. Actual Results: no cursor change! Expected Results: cursor change to 'watch' (the hourglass)

old build, but still an issue... more of an enhancement thought

adding 4xp keyword, updating component and owner.

This is a pretty standard behavior and a very important feedback element for users. If the machine doesn't respond *immediately* there's no way to know whether the action was recognized. Consequently, a lot of users will try again or will try something else. The throbber isn't good enough, either, because more often than not, that's not where the user clicked, so that's not what they're looking at. It's not that hard to change a cursor...

Since it sounds like an actual bug (per Brendan's comments), reassigning to Don, and upping severity to 'Normal'.

QA Assigning non-confidential New/Assigned User Interface: Design Feedback bugs to Matthew Thomas (mpt@mailandnews.com). Matthew Thomas is now the QA owner for the User Interface: Design Feedback component. (Bugs that involve UI issues in the Netscape-branded Mozilla browser should continue be QA assigned to elig@netscape.com.)

This is crucial to users' impressions of how fast the browser is generally. I have seen many occasions where novice- to intermediate-level Internet Explorer users click a link and (due to an occasional IE bug) the cursor isn't changed to a busy one. They think nothing has happened, so they click the link again. And again, and again. Each time they click, of course, the browser restarts the connection process from scratch, resulting in a much longer time taken to actually open the page. Mozilla's throbber and indeterminate progress indicators are more obvious than those in IE, but it's still better to get immediate feedback right where you clicked, i.e. in the cursor. Nominating for nsbeta2, since it affects user perception of speed and since it should be fairly trivial to implement. Note that you should use the half-busy cursor here (the combination pointer and watch/hourglass), to show that it is still possible to click on another link in the page while Mozilla is trying to connect to the first link.

Putting on [nsbeta2-] radar. Not critical to beta2.

Reassigning as per Don

I don't know how to do this. I think there's already a style rule in global that may specify this, but if it's not applying, I would have no idea why. Back to don.

Move to M20 target milestone.

This bug could also lead a user into submitting form data twice, which could be *really* bad. Upping to major severity.

Matt, can you figure out how to implement this? If not, please find the appropriate person to doom. I agree with Matthew Thomas, there's a big danger if we're not providing the right kind of feedback here.

Question: How long should the cursor stay busy? Should it stay busy while the entire page is loading, or just until it gets it from the network and the page starts loading? I'm attaching a patch for this that does the former. I couldn't get CSS to work directly, instead I used the window.setCursor method, passing "spinning" as a parameter (spinning is a new cursor in CSS3 and represents the hourglass/arrow combination). It's not perfect -- the browser window sometimes tries to set the cursor back to an arrow... but I think it's a lot better than nothing.

(CCing Tim because I don't see him here.) When does this patch make the cursor kick in? It should be right after the browser has decided to load a page, and right before the DNS lookup starts. Is that correct?

Right now it basically turns the hourglass/arrow on and off at the same time the throbber is turned on and off.

Sorry, Tim, didn't answer your questions. * IE uses the spinning cursor until the moment the new page starts rendering. However, my feeling is that you should continue it until the new page finishes loading. (That way it gives a subtle indication that incremental reflows might cause the layout of the page to change.) * Using the mixed pointer-and-hourglass is correct (because you can do other things while the page is loading, e.g. click on another link). W.r.t `the browser window sometimes tries to set the cursor back to an arrow': what should be happening is that the `spinning' cursor is used *whenever the ordinary arrow cursor would normally be used*, and only in those cases. In all places where other cursors are specified (sidebar grippy, page elements with :cursor style, etc), those specified cursors should still be applied.

Yep, tying it to the throbber makes sense. Duh, I didn't think of that. :-)

Could this be causing some of the double bugs? I'm talking about the cases where there are two identical bugs submitted at allmost the same time. Fixing this could cut down on the number of dupes.

The patch turns the cursor to the arrow/hourglass combo over ALL elements in the page (and browser), which I think makes sense. So if a new page is in the middle of loading and you mouse over a link, it won't change it to the hand pointer. Once the page is loaded of course, it changes it back to "auto", so the cursors then work as expected. I saw the "patch" keyword on this briefly, but it disappeared for some reason. Matt, are you working on this? Or perhaps Ben can check this in.

Hmmm, ok. This will mean you don't (for example) get the double-arrows cursor when mousing over the sidebar grippy while a page is loading ... but loading feedback is more important than that, I guess. [Sorry, I think I removed the patch keyword by mistake in a mid-air collision. Reinstating.]

Nav triage team: [nsbeta3+] please double plus this.

I have this in my tree and can check in the patch you submitted. It has been working pretty well for the most part. Waiting for the green to go normal.

I've been running with it for a while too and I've noticed that some sites just want to reset the cursor all of the time. For example, http://www.news.com . My patch tries to account for this somewhat, but it's not completely successful. Also, I kind of miss the hand cursor feedback when moving over a hyperlink while a page is loading. Ideally it would only show the hourglass when hovering over the background of the page. Both of these probably require a more complicated solution than a few lines of Javascript code. Some cursor feedback is better than nothing, I suppose, and IE and Nav4 aren't perfect either. I haven't tested this on a Mac or Unix, by the way.

patch checked in. Sorry that took so long. I do see the problem of the hourglass getting reset all the time. We should file another bug for that.

I think it's pretty bad that the cursor never changes where it should now after you've clicked a link...Is there _any_ better solution to this?

Sorry, I'm reopening this. We'll have to find a better way to do it; I think it's a major loss of feedback that the cursor doesn't change to a hand when over a link once you start navigating somewhere. It seems as if you simply can't click the link at all, which isn't true (the same goes for many other parts of the UI).

I tend to agree that hyperlink cursor feedback is important. In the long term, any time the cursor is set to an arrow, it should use the arrow/hourglass combo instead IF the page is currently loading. This bug may need to be reassigned; I'm not sure who owns the various pieces of code that modify the cursor. In the short term I might be able to change this to match IE's behavior, which only uses the arrow/hourglass in the content area UNTIL the new page starts loading (also, it uses the arrow/hourglass in the chrome area the entire time). I still think it's nice to have the arrow/hourglass feedback while the page is loading, but the current solution that hides the hyperlink feedback is less than ideal. Also, having this in JavaScript means embedded Gecko applications don't get this basic behavior automatically.

See my comments on 2000-07-19: the partially-busy cursor should be `used *whenever the ordinary arrow cursor would normally be used*, and only in those cases'. Which means, not in the cases where the pointing hand cursor would be used. Is it possible just to piggy-back the case where the ordinary arrow would be used, and replace it with the partially-busy cursor?

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

Right now Linux is using XC_exchange instead of XC_watch, which I find to be ugly (see bug 48804). This just changed a few days ago. Will a fix for this bug do something about that? (my guess is yes, so I haven't filed a bug on it.)

In order to replace the arrow cursor with a hourglass/arrow efficiently and in all cases, the C++ code needs to be modified. If Netscape has the resources to do this, that would be great. I might buy a Visual C++ compiler soon to see if I could work on it myself, taking a deep breath and saying a few prayers beforehand :) I'm presuming what needs to happen is to add some code around here... http://lxr.mozilla.org/seamonkey/source/layout/events/src/nsEventStateManager.cp p#1252 ...which adds a special case for using the busy cursor in place of the standard arrow whenever the page is loading. In order to do that, you need to somehow communicate with the document in order to retrieve the busy state (not sure quite how to do that yet). Finally you need to make sure the cursor is appropriately updated at the beginning and end of the document load. Sadly I can't mentally compile and execute so this is only a guess.

Regarding the Unix cursor. I agree the cursor used for CSS3 "spinning" in Unix is a bit ugly. The situation will be improved if the hyperlink cursor is used for mouseover hyperlinks, but it would still be shown on the page background. If you think CSS3 "spinning" should be a different cursor (such as the watch, the same as CSS3 "wait"), you should probably file a separate bug.

I've been trying to modify the fix for the problem that the cursor does not change once a new page is loaded. I don't have time to change the c++ code the change the cursor in ever possible case but it would be good at least cover the case you have been talking about. Doing the other would be a toolkit bug.

Looks like Mac (bug 49173) and Unix people (bug 48839) really don't like the busy cursor appearance. Even if we handle the case for hyperlinks, I'm guessing that the cursor may still be annoying for platforms with less pretty cursors than Windows. I suggest we check the window.navigator.platform string at startup to either turn off, or special case this behavior for other platforms. The values are "Windows", "OS/2", "Macintosh", and "X11" (Unix). I'm not sure what the current behavior is on the Mac and Unix for Navigator4, and IE5/Mac. Perhaps we can use "wait" (the watch) instead of "spinning" for those platforms. And/or consider just using the busy cursor until the new page begins loading.

IMO we shouldn't special-case it, we should leave it how it is and get bug 48839 and bug 49173 fixed. Using the `wait' cursor would be misleading the user, because it would be implying that user input wasn't possible when it actually was. I don't understand why this bug blocks bug 48839, by the way.

nav triage team: we need to have a final word on what the "right" behavior is. Matt, Gable and German should talk about this - reassign to johng for decision. We need to make this decision for all related bugs dealing with curser wait indications.

I'm currently testing some C++ code I wrote to support busy cursors in all DocShells. It seems to work fairly well so far. It basically covers the embedding case, chrome windows, browser windows -- anything that uses nsDocShell to load content. For example, the busy cursor is displayed when switching between the very slow-loading Preferences panels. The cursor behavior is as follows... o Not Busy: - Appropriate cursor is used when hovering over elements. o Busy... - BEFORE a new page starts loading (i.e. connecting to the network): "Spinning" cursor is used over ALL elements. This is necessary so when you first click on a link, you know immediately that the new page is loading w/o having to move the mouse off the link. Cursor feedback here isn't really too important since the existing page is going away. Exact same behavior as IE 5. - AFTER the page starts loading: "Spinning" cursor only used where the arrow/default cursor would otherwise be used. The hand cursor is shown over hyperlinks, for example. (I'm wondering whether the "spinning" cursor should also take the place of the I-beam cursor that is used over selectable web page text? Otherwise I'm not sure if having busy cursor feedback during page load is even that useful, since much of a page is often made up of text)

I agree with Matthew Thomas it should be the partial wait cursor substitutung where you would otherwise see the arrow pointer arrow, and only in those areas. Meaning that hor/vert grippies should still show the e-w and n-s resize cursors even while loading. That is the point of giving loading feedback while stating that some areas are still clickable.

Tim, your wondering is correct -- `spinning' should be used during the page load even where you are over selectable text. Incremental reflows mean that the text may move as the user tries to select it, and using `spinning' here helps warns her of this. (But if she starts dragging to make a selection while the page is still loading, the I-beam cursor should be used *during the drag*, as it would be in the normal case.) Johng: German has commented here, which presumably means you have had your discussion -- so do you want to reassign this back to a programmer?

The patch seems fine to me, but please ensure that it works with situations such as the user stopping a load, a load timing out etc. The cursor should also revert to the normal shape when the pointer is over the scrollbar.

I'm fairly certain that the busy cursor will work in those cases, because it responds to the same events as the throbber does. The busy cursor should still be used for the scrollbar since it's normally an arrow... I'm not sure how difficult it would be to use the I-beam while dragging but not otherwise. I'll look into it.

Not critical to Netscape 6 ship. Moving to P3. Adding [PDTP3]

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

Renominating for beta3. Days before nsbeta3 and not even assigned to an engineer yet, this isn't a beta3 stop ship. it would be appreciated if someone could review those patches, however...cc saari for hopeful patch evalation.

JohnG, does Germans' 2000-08-24 comment represent the decision of Matt, Gable, and German Inc.? If so, please reassign to a programmer. If not, please tell us what the delay is. It would be nice if this could get in for M18. Thanks.

German's decision is the right one. Reassigning to Don Melton to get help with triaging. Do we need to fix this for RTM? This does effect the user's perceived performance of the product.

We're not gonna take this for beta 3. Maybe RTM. Ben, are these patches cool to take at this point?

Marking nsbeta3-.

sending to jud. jud, does your group own this code? I tried this patch and it works fine for me.

PDT marking [rtm-] for N6. Doesn't seem very serious.

Tim, a couple of comments on the patch. 1. Change the style from busyNotBusy to BUSY_FLAGS_NONE etc. and from signed to unsigned longs. 2. Consider using flag combinations to make up the various states. e.g. let people test for the BUSY_FLAGS_BUSY bit instead of having to compare against every busy state, busyPageLoad OR busyBeforePageLoad. Otherwise it looks fine.

Tim, mind producing an updated version of the patch that addresses Adam's comments so we can get it in?

Hmm.... So, I made a patch updated to the tip that addresses most of Adam's comments, but I'm not seeing the exact behavior Tim outlined in his 8/24 comment. Instead, although the cursor properly changes in the busyBeforePageLoad state, it quickly changes back to the standard cursor the second you move the mouse. Any ideas? Some stuff changed in OnStateChange() since this patch was produced (STATE_IS_NETWORK -> STATE_IS_DOCUMENT, for example, although that had no effect). I'll post the updated patch if you want, but it's kind of useless with the aforementioned problem.

-> adam

really -> adam

Blake, can you provide me with your later patch? Thanks

I've updated the patch slightly to work with the latest source code and to pick up some of the changes I suggested. I'll test it a little more tomorrow and if no one has any objections it will be submitted for review.

sorry to be a grouch... + } + else if ((aStateFlags & STATE_STOP) && (aStateFlags & STATE_IS_NETWORK)) + { ... + if(mainWidget) + { + mainWidget->SetCursor(eCursor_standard); + } let's pick an indentation style and maintain it. the one i prefer is: if (cond) { statement; } else { statement; } <!-- //the only case that does not follow this style is functions: void functionName ( ... ) { body; } --> i think that: + else if ((aStateFlags & STATE_STOP) && (aStateFlags & STATE_IS_NETWORK)) can be rewritten as + else if (aStateFlags & STATE_STOP && aStateFlags & STATE_IS_NETWORK) however i'll leave that for brendan

timeless wrote: >i think that: >+ else if ((aStateFlags & STATE_STOP) && (aStateFlags & STATE_IS_NETWORK)) >can be rewritten as >+ else if (aStateFlags & STATE_STOP && aStateFlags & STATE_IS_NETWORK) >however i'll leave that for brendan It's not a big deal, but I prefer unnecessary parenthesization of & against && for readability's sake (without the parens, the &&&s are hard to tell apart). However, there's no need for a separate mask and test, and the branch implied by &&, to test whether two bits are set: + else if ((aStateFlags & (STATE_STOP|STATE_IS_NETWORK)) == (STATE_STOP|STATE_IS_NETWORK)) Notice that parens are required now, due to C and C++'s notorious botched precedence of equality operators as higher than the bitwise logicals. Even shorter and faster: + else if ((~aStateFlags & (STATE_STOP|STATE_IS_NETWORK)) == 0) /be

I've updated the patch to include suggestions. I consider the original bitfield testing to be more readable but I've changed it since a more efficient way was suggested. Also note that docshell is a mess of coding conventions so no style would be correct for this file (many blocks are indented by 3 spaces!) but I've changed the brace style anyway. This patch is now ready for review.

filed bug 66434 for cleanup of nsDocShell.cpp

Adam: did you mean to leave this brace-style instance (and one like it soon after) alone? ! if(mainWidget) ! { ! mainWidget->SetCursor(eCursor_spinning); ! } Also, please use diff -u. Sorry to nag, I don't have more to add right now. As http://www.mozilla.org/hacking/reviewers.html, Rule 20, makes clear, bad or no ownership made manifest by crappy/ugly/inconsistent style is a bug to be fixed, incrementally or all at once, by new and better owners. You should go strong, get rid of Travis's evil Fibonacci-number indentation, and be a righteous owner. /be

Fix is checked in now. Coding style issues in docshell are covered by that other bug.

Who let this go by: if ((~aStateFlags & (STATE_START | STATE_IS_NETWORK)) == 0) which is equivalent to the much more easily comprehended if (aStateFlags & (STATE_START | STATE_IS_NETWORK))

No it isn't, if ((~aStateFlags & (STATE_START | STATE_IS_NETWORK)) == 0) Is equivalent to: if ((aStateFlags & STATE_START) && (aStateFlags & STATE_IS_NETWORK)) I changed the expression from the latter to the former following comments from Brendan. See above.

Oops, you're right. However, I find the if ((aStateFlags & (STATE_STOP|STATE_IS_NETWORK)) == (STATE_STOP| STATE_IS_NETWORK)) or if ((aStateFlags & STATE_START) && (aStateFlags & STATE_IS_NETWORK)) forms much easier to comprehend. Smaller brain-print. (BTW, this error I made does not affect my patch in bug 67084, where I note that all this needs to be refactored anyway).