Closed Bug 40838 Opened 24 years ago Closed 18 years ago

disallowing "change status bar text" often leaves status bar empty for links (onmouseover="return true;" --> status bar doesn't show url for links)

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha4

People

(Reporter: xalkina, Assigned: florian)

References

(Depends on 2 open bugs, )

Details

(Keywords: testcase)

Attachments

(6 files, 6 obsolete files)

after entering service and seeing the tell-a-friend button in bottom "mood-bar" all mood buttons, when onmouseovering show their url in status bar taf, the last one, doesn't show anything --just "document done" tested on linux-2000052520
fixing url so it isn't a porn site :)
Attached file simpler testcase (deleted) —
confirming (2000 052615 on win98), updating summary, etc. related: bug 38380 (another onmouseover="return true;" bug)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
OS: Linux → All
Hardware: PC → All
Summary: onmouseover not showing target url for one link → onmouseover="return true;" --> status bar doesn't show url for links
Okay, I'm not entirely sure I understand the issue here but the all the test cases seem to be performing as I would expect. The test case which has 'return true' for all of the mouseover handlers will correctly not show a url. In the other test cases which don't have 'return true' in the mouseovers we correctly show the url. I'm going to mark this WORKSFORME but if I'm missing something here please reopen this.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → WORKSFORME
Still doesn't work for me! 4.73 on the same page shows urls in status bar, linux/2000061220 mozilla does nothing. Checked the source, there's no return true, there aren't any evetns specified for the moods at all!
>The test case which has 'return true' for all of the mouseover handlers will >correctly not show a url. In the other test cases which don't have 'return >true' in the mouseovers we correctly show the url. You seem to agree with what mozilla is doing. Why is what mozilla is doing correct, though?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
That is how onmouseover handlers work. If you have an onmouseover handler which ends with a 'return true' the browser will not show the URL for the link. If you have an onmouseover handler which ends without a return value or with 'return false' the browser will show the url. This has been true since I believe Netscape 2.0, definately since 3.0. I'm marking this fixed again now, else I will likely forget to close it again. If you think the browser is not behaving as I've described above please reopen it.
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
Is it just me? The page does NOT have onmouseover events specified, which means, mozilla HAS TO SHOW the url when mouse is over the images in the bottom part. However, mozilla DOES NOT show them. Mind you, this IS a bug. Currently using 061311 in linux
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Well there is a mouseover but its on the image. Which I guess is the real question, now that images can get mouseovers and those events bubble up to the link do we keep using the same rule that 'return true' prevents the display of the url?
Oops! You're right! I just checked the some of the first images that don't have onmouseovers. Earlier builds of mozilla used to do the same thing with all the buttons. Sorry for shouting!...
This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.
Target Milestone: --- → Future
Mass update: changing qacontact to ckritzer@netscape.com
QA Contact: janc → ckritzer
Updating QA Contact.
QA Contact: ckritzer → lorca
I modified the attachment demonstrating the problem this bug deals with. This bug is very confusing so far... I think the main question comes down to whether onmouseover in an image should remove the url from the link. We admit that when onmouseover is in a link there should be no URL. This is the case in the examples. I'm fine with the onmouse over in an image tag affecting the "URL in status bar" of its parent. As long as the text in the first example retains the URL, then the parent-child relationship works. If a user hovers over the image with a "return true" statement then all status messages from its parents should also be affected. Hopefully this bug can be resolved since its workings are what is expected. (BTW: is there a W3C standard for this? I couldn't find one specifically)
Reassigning QA Contact for all open and unverified bugs previously under Lorca's care to Gerardo as per phone conversation this morning.
QA Contact: lorca → gerardok
QA contact updated
QA Contact: gerardok → madhur
This bug no longer exists. The testcases now show the URL for all links whether images or not. onmouseover="return true;" has no effect on the status bar. (Which is as I now see it, is the proper way to go. unless we specifically mention window.status there should be no change to the status bar.) Marking WORKSFORME if anyone objects, please reopen.
Status: REOPENED → RESOLVED
Closed: 24 years ago23 years ago
Resolution: --- → WORKSFORME
QA Contact: madhur → rakeshmishra
Reopening (was just about to file this one myself). Using build 2002050306 from the 1.0 branch, none of the links in attachment 9272 [details] show the URL in the status bar when I hover over them.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
I confirm (today's build of the 1.0.0 branch under Linux)
Found it on http://manga.flyswatter.ca/main.html too. It is annoying - it looks like mozilla is stuck. Also, I have "Change status bar text" disabled on the prefs, so the site shouldn't be able to do anything which would affect the status bar. Including "return true". I think the page is using the "return true" to prevent the browser from overwriting the text it just set using window.status: onmouseover="window.status='** page policies **' ;return true" Else it would be overwritten by the URL.
I also browse with the "Change status bar text" pref (dom.disable_window_status_change) enabled (i.e. don't let scripts change the status bar text) and to me the expected behavior should then be that the status bar will only ever show the URL when hovering over a link. It seems that what happens is that setting window.status is a no-op, but the event handler is allowed to return true, ensuring that no URL appears. I would think that such behavior is still allowing a script to change the status bar, even if the change is effectively setting the status bar text to nothing. Is a suitable fix to make every call that sets window.status while the above pref is enabled instead set the status bar text to the underlying URL? Then the return value from the event handler is irrelevant and any other processing done in the event handler can remain unaffected.
*** Bug 166923 has been marked as a duplicate of this bug. ***
My remark from bug #166923: Strangely. The link URL *is* shown correctly if "capability.policy.default.Window.status" is set to "noAccess" in prefs.js. I cannot image this is by intention. If it is, I consider it wrong.
QA Contact: rakeshmishra → trix
*** Bug 135849 has been marked as a duplicate of this bug. ***
*** Bug 177928 has been marked as a duplicate of this bug. ***
> Strangely. The link URL *is* shown correctly if > "capability.policy.default.Window.status" is set to "noAccess" in prefs.js. So can't we do away with dom.disable_window_status_change, and have the UI set this the CAPS one? It seems to do what everyone wants. Having a blank sbar is worse than letting the link set it. At least that way you can see the section name usually.
> So can't we do away with dom.disable_window_status_change, and have the UI set > this the CAPS one? Not until bug 122866 is fixed. CAPS currently throws an exception, causing the script to stop executing. See discussion in bug 117707.
*** Bug 190609 has been marked as a duplicate of this bug. ***
I guess taht the caps way _only_ works _because_ it throws, because the JS will not be executed further and the return will not be reached. To solve this - how about moving the generic HandleDOMEvent call at http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsGenericHTMLElement.cpp#1416 to the end of the function, so the statusbar will always contain a status text?
*** Bug 203970 has been marked as a duplicate of this bug. ***
There is some quite important site which shows the problem, search results on AlltheWeb.com, e.g.: http://www.alltheweb.com/search?q=bugzilla pi
To clarify what pi said, this bug affects Alltheweb *if you have disallowed scripts changing window.status*.
Summary: onmouseover="return true;" --> status bar doesn't show url for links → disallowing "change status bar text" often leaves status bar empty for links (onmouseover="return true;" --> status bar doesn't show url for links)
.
Assignee: joki → saari
Status: REOPENED → NEW
QA Contact: trix → ian
There is another page that demonstrates this issue: http://java.sun.com/j2se/1.4.1/download.html On that page there seem to be no JavaScript mouse hooks at all, but when whe mouse poiter moves over the "DOWNLOAD" image, the status bar shows nothing. Actually, it is not really a link, it's some kind of "form submit button", so is that a bug or not? Anyway, it would be nice to see something useful on the status bar, so even if it is supposed behavior, please consider it as a feature request.
Same problem on several pages. (I checked some of the examples with my Moz 1.5 rc W98 and they "worked" as described by the contributors. (text and image links not working, flash neither, but that's flash of course.) I also have disabled JavaScript to change status bar text etc.. Copying link location & pasting in editor/url-field works so it is a matter of beauty but it would still be fine to have that one fixed someday. By the way: Those java sun downloadbuttons are small gifs and there is no copy link location option for them.
Comment on attachment 136498 [details] Different link behaviours, even after prefs.js setting Changing the 'capability.policy.default.Window.status' fixes the behaviour if there are more than one JS statements in the mouseover action. If it only contains a 'return true' the link is still hidden.
Norbert: that's because capability.policy.default.Window.status makes "window.status='foo';" throw an exception (bug 122866), as explained in comment 32.
*** Bug 227159 has been marked as a duplicate of this bug. ***
Perhaps this is obvious, but from the bug I just duped (bug 227159), the same behavior can be seen in Firebird if you disable JavaScript entirely. Also, is Saari still the correct assignee for this bug, or should it be reassigned to the default address?
Assignee: saari → events
*** Bug 251166 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.0?
*** Bug 266034 has been marked as a duplicate of this bug. ***
(In reply to comment #38) > Copying link location & pasting in editor/url-field works so it is a matter of > beauty but it would still be fine to have that one fixed someday. Another work-around: right-clicking the link shows the URL in the status bar, until the mouse moves off the link.
*** Bug 267337 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.0? → blocking-aviary1.0-
*** Bug 268873 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1?
"If you have an onmouseover handler which ends with a 'return true' the browser will not show the URL for the link... This has been true since I believe Netscape 2.0, definately since 3.0." From a programming point of view, can anyone say why this is rational behaviour? The status bar isn't even mentioned in the javascript, just 'return true' which makes absolutely no sense to me. FWIW I'll second comment 24.
*** Bug 280929 has been marked as a duplicate of this bug. ***
*** Bug 287500 has been marked as a duplicate of this bug. ***
*** Bug 289354 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1? → blocking-aviary1.1-
*** Bug 299765 has been marked as a duplicate of this bug. ***
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
Attachment #195783 - Flags: review?(jst)
Comment on attachment 195783 [details] [diff] [review] patch v1 if ((*aEventStatus == nsEventStatus_eIgnore || - (*aEventStatus != nsEventStatus_eConsumeNoDefault && + ((*aEventStatus != nsEventStatus_eConsumeNoDefault || + nsContentUtils::GetBoolPref("dom.disable_window_status_change")) && (aEvent->message == NS_MOUSE_ENTER_SYNTH || aEvent->message == NS_MOUSE_EXIT_SYNTH))) && !(aFlags & NS_EVENT_FLAG_CAPTURE) && !(aFlags & NS_EVENT_FLAG_SYSTEM_EVENT)) { The order inside the part of the condition you're changing could be changed so that we don't check the pref unless we're dealing with a enter or exit event since the pref check isn't all that cheap. r=jst, bz should sr once you have an updated patch.
Attachment #195783 - Flags: review?(jst) → review+
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
Attachment #195783 - Attachment is obsolete: true
Attachment #196055 - Flags: superreview?(bzbarsky)
Attachment #196055 - Flags: review+
So: 1) Why does this work? 2) Why is this change only needed for HTML?
(In reply to comment #57) > So: > > 1) Why does this work? The function HandleDOMEventForAnchors tries script event handlers first. If the script returned true, *aEventStatus is set to nsEventStatus_eConsumeNoDefault before we get to the test my patch changes. If the value of *aEventStatus was nsEventStatus_eConsumeNoDefault the link url wasn't put to the status bar. If the pref "dom.disable_window_status_change" is set, we should write the url to the status bar even when the value of *aEventStatus is nsEventStatus_eConsumeNoDefault. That's what it does with the patch. > 2) Why is this change only needed for HTML? I don't know if this change is only needed for HTML. I tried but couldn't reproduce the bug on svg:a. Anyway, it's for HTML that this bug is the more annoying.
> If the pref "dom.disable_window_status_change" is set, we should write the url > to the status bar even when the value of *aEventStatus is > nsEventStatus_eConsumeNoDefault. Why? Things other than user script can set the event to be consumed with no default... Do we still want to show the text then? That is, are we making the (unwarranted) assumption that the only thing cancelling events is user script? For example, if an extension attaches a handler to an <a> that sets window.status (which it can do no matter what the value of the " "dom.disable_window_status_change" preference) and returns true, then we'll ignore the fact that it returned "true" and still show the link URI instead of the text the extension set, won't we? To test this easily, just remove the CanSetProperty check in nsGlobalWindow::SetStatus, apply your patch, set the pref, then see what the behavior is for content script that sets the status on links on mouseover. Perhaps the right solution would rather be for user script to not be able to cancel the mouseover event if this pref is true? That also has issues, I suspect... > I tried but couldn't reproduce the bug on svg:a. Here's a simple non-HTML testcase that shows the bug: data:text/xml,<html:html xmlns:html="http://www.w3.org/1999/xhtml"><html:span onmouseover="return true;"><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="http://www.mozilla.org" xlink:type="simple">This is a link to mozilla.org; hover it and see what happens</a></html:span></html:html> A similar testcase with <svg:a> would show the same problem, since that uses XLink on the inside. > Anyway, it's for HTML that this bug is the more annoying. This is one of those things where if we don't fix it for non-HTML, we'll just have to go through all this again in a year or two. I'd like to see a patch that fixes both HTML and non-HTML at once.
One option is to set the status bar text before the event is fired.
That would probably make the most sense, in fact....
Er... Except generally speaking, if the event's preventDefault() is called we arguably should not show the status bar text, since that is the event's default action.
> Er... Except generally speaking, if the event's preventDefault() is called we > arguably should not show the status bar text, since that is the event's default > action. Only be convention. We could change that easily -- most (all?) of the time if someone is going to cancel the event to prevent us from updating the status bar, it's because they want to do it themselves. In which case they'll do it and overwrite our text and everything will be as expected.
Comment on attachment 196055 [details] [diff] [review] patch v2 Fair enough. Let's do that, then. I like that better than the proposal here. Please also fix the nsXMLElement code, and in general any code that calls OnOverLink.
Attachment #196055 - Flags: superreview?(bzbarsky) → superreview-
Florian, are you willing to take assignment of this bug? /be
Attached patch patch v3 (obsolete) (deleted) — Splinter Review
New patch following the idea "set the status bar text before the event is fired" (comment 60). My patch may be wrong since I'm not sure I understand every line I moved.
Assignee: events → f.qu
Attachment #196055 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #201286 - Flags: superreview?(bzbarsky)
That patch needs review too, not just sr. In any case, I won't be able to look at this any time soon (weeks, at least). I'd strongly suggest finding another reviewer...
Attachment #201286 - Flags: superreview?(peterv)
Attachment #201286 - Flags: superreview?(bzbarsky)
Attachment #201286 - Flags: review?(jst)
There is other odd behavoir going on, besides the mouseover behavoir that attachment (id=136498) of Comment #39 was created for. Click either link that isn't displying in the status bar, will cause it to display, BUT only upon the 1st click. Thereafter, clicks on the link will NOT display. Only after clicking upon Anything else, the body, anything, another link. ... Then once again the link will display on the 1st click. Behavoir is better seen if href="javascript:void('')". Change status bar text has no effect on this behavoir. This displaying upon the click is mention in 158591 which is a possible duplicate. They did not mention this "once only" displaying. I would think if is suppose to happen the 1st time, it should happen the 2nd and 3rd, etc. With a live link, one does not get to observe this 2nd, 3rd, etc. click behavoir.
(In reply to comment #68) It's happening from the focus event, so that's why you don't see it on the second click. You have to un-focus the link and then re-focus it to see the behavior again. You can verify this by catching onfocus on the link and doing preventDefault(). This prevents the link from getting focus, and also prevents the "first click" from displaying the URL.
*** Bug 333793 has been marked as a duplicate of this bug. ***
*** Bug 158591 has been marked as a duplicate of this bug. ***
Flags: blocking1.9?
Attachment #201286 - Flags: review?(jst) → review?(Olli.Pettay)
(In reply to comment #66) > Created an attachment (id=201286) [details] > patch v3 > Could you perhaps update the patch, since it is from pre-event-dispatching-rewrite era.
Comment on attachment 201286 [details] [diff] [review] patch v3 Clearing r?. Waiting for an updated patch.
Attachment #201286 - Flags: review?(Olli.Pettay)
Attached patch patch v4 (obsolete) (deleted) — Splinter Review
Attachment #201286 - Attachment is obsolete: true
Attachment #262435 - Flags: superreview?(bzbarsky)
Attachment #262435 - Flags: review?(Olli.Pettay)
Attachment #201286 - Flags: superreview?(peterv)
Comment on attachment 262435 [details] [diff] [review] patch v4 For future reference, please use more context and the -p option when diffing. That makes patches a LOT easier to review. -up8 is a good set of options. >Index: base/src/nsGenericElement.cpp >+nsGenericElement::PreHandleEventForLinks(nsEventChainPreVisitor& aVisitor) >+ if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault || >+ !NS_IS_TRUSTED_EVENT(aVisitor.mEvent) || >+ !aVisitor.mPresContext) { >+ return NS_OK; >+ } Does that condition need to match the similar one in PostHandleEventForLinks? If so, document so in both places. >+ // Make sure we actually are a link before continuing Same for this condition. In fact, maybe this chunk of code should be factored out into a method called from both places. >+ switch (aVisitor.mEvent->message) { Right before that switch, I would put in a nice comment explaining why all this is being done in PreHandleEvent. >+ case NS_MOUSE_EXIT_SYNTH: >+ aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault; >+ rv = LeaveLink(aVisitor.mPresContext); >+ break; >+ >+ case NS_BLUR_CONTENT: >+ rv = LeaveLink(aVisitor.mPresContext); >+ break; I know you just moved this, but can't you use fall-through here too? >Index: html/content/src/nsGenericHTMLElement.cpp >+nsGenericHTMLElement::PreHandleEventForAnchors(nsEventChainPreVisitor& aVisitor) >+ nsGenericElement::doPreHandleEvent(this, aVisitor); Why not just call nsGenericElement::PreHandleEvent here? I'd prefer this. And probably bail out if that returns error. This function copies a whole bunch of logic from nsGenericHTMLElement::PostHandleEventForAnchors. Please factor this out into a function instead. We really don't want those to get out of sync. >Index: svg/content/src/nsSVGAElement.cpp >+nsSVGAElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) >+ nsGenericElement::doPreHandleEvent(this, aVisitor); Again, why not PreHandleEvent()? And why ignore the rv? >Index: xml/content/src/nsXMLElement.cpp >+nsXMLElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) >+ nsGenericElement::doPreHandleEvent(this, aVisitor); And here.
Attachment #262435 - Flags: superreview?(bzbarsky) → superreview-
Attached patch patch v5 (obsolete) (deleted) — Splinter Review
Attachment #262435 - Attachment is obsolete: true
Attachment #262459 - Flags: superreview?(bzbarsky)
Attachment #262459 - Flags: review?(Olli.Pettay)
Attachment #262435 - Flags: review?(Olli.Pettay)
Comment on attachment 262459 [details] [diff] [review] patch v5 >Index: base/src/nsGenericElement.cpp >+nsGenericElement::CheckHandleEventForLinksPrecondition(nsEventChainVisitor& aVisitor, >+ if (!IsLink(aURI)) { >+ return PR_FALSE; >+ } >+ >+ return PR_TRUE; Isn't that the same thing as |return IsLink(aURI);|? >+nsGenericElement::PreHandleEventForLinks(nsEventChainPreVisitor& aVisitor) >+ // updated even if the event is consummed before we have a chance to set it. "consumed". >Index: base/src/nsGenericElement.h >+ * Check that we meet the conditions to handle a link event >+ * and that we are actually on a link. >+ * >+ * @param aVisitor event visitor >+ * @param aURI the uri of the link [OUT] Document what the return value means? How is aURI related to the return value? Can it be null if the return value is PR_TRUE? >+ * Handle default actions for link event if the event isn't consummed yet. "consumed" >Index: html/content/src/nsGenericHTMLElement.cpp > if (target && IsArea(target) && !IsArea(this)) { >+ return PR_FALSE; >+ } >+ >+ return PR_TRUE; return !target || !IsArea(target) || IsArea(this); >+nsGenericHTMLElement::PreHandleEventForAnchors(nsEventChainPreVisitor& aVisitor) >+ if (rv != NS_OK) { >+ return rv; > } NS_ENSURE_SUCCESS(rv, rv); please. >+ if (!CheckHandleEventForAnchorsPreconditions(aVisitor)) >+ return NS_OK; Please put curlies around the body of the if. >+nsGenericHTMLElement::PostHandleEventForAnchors(nsEventChainPostVisitor& aVisitor) >+ if (!CheckHandleEventForAnchorsPreconditions(aVisitor)) >+ return NS_OK; Same here. >Index: html/content/src/nsGenericHTMLElement.h >+ PRBool CheckHandleEventForAnchorsPreconditions(nsEventChainVisitor& aVisitor); Document what this function does? Specifically, what does a PR_TRUE or PR_FALSE return mean? >Index: svg/content/src/nsSVGAElement.cpp >+nsSVGAElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) >+ if (rv != NS_OK) { >+ return rv; >+ } NS_ENSURE_SUCCESS(rv, rv); >Index: xml/content/src/nsXMLElement.cpp >+nsXMLElement::PreHandleEvent(nsEventChainPreVisitor& aVisitor) >+ if (rv != NS_OK) { >+ return rv; >+ } NS_ENSURE_SUCCESS(rv, rv); sr=bzbarsky with those nits picked.
Attachment #262459 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 262459 [details] [diff] [review] patch v5 Fix bz's comments, r=me
Attachment #262459 - Flags: review?(Olli.Pettay) → review+
Attached patch patch v6 (obsolete) (deleted) — Splinter Review
Should be ready for checkin.
Attachment #262459 - Attachment is obsolete: true
Attached patch patch v6 bis (deleted) — Splinter Review
Attachment #262474 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 23 years ago18 years ago
Resolution: --- → FIXED
Target Milestone: Future → mozilla1.9alpha4
Depends on: 731106
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: