Closed Bug 372773 Opened 14 years ago Closed 13 years ago

Tab favicon should have the grab cursor on hover

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3 alpha8

People

(Reporter: pile0nades, Assigned: dao)

References

()

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2

Tabs can be dragged into bookmarks the same way the urlbar favicon is, but only the urlbar favicon has the grab cursor indicating draggability. The tab favicon should have the grab cursor too.

The URL is a Stylish userstyle to implement this.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
We had some builds of Bon Echo working that way for a while but decided to pull it before the release. If I recall correctly, the decision had to do with the hand cursor not really symbolizing dragging an object rather than dragging, for example a page to move around, but it would be better to be consistent.
(In reply to comment #2)
> We had some builds of Bon Echo working that way for a while but decided to pull
> it before the release.

That will probably have to change, in case we remove the favicon from the location bar.
Blocks: 366797
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch patch, icon only (obsolete) — Splinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #266040 - Attachment description: patch → patch, icon only
Attached patch patch, whole tab (obsolete) — Splinter Review
Attachment #266345 - Flags: ui-review?(beltzner)
Blocks: 382220
No longer blocks: 366797
Comment on attachment 266345 [details] [diff] [review]
patch, whole tab

Not on the whole tab - the primary interaction for tabs is selection, not dragging/moving.
Attachment #266345 - Flags: ui-review?(beltzner) → ui-review-
Attachment #266040 - Flags: ui-review?(beltzner)
Comment on attachment 266040 [details] [diff] [review]
patch, icon only

I can't decide if this is weird or not. Might end up asking to back it out, but let's take a peek.
Attachment #266040 - Flags: ui-review?(beltzner) → ui-review+
Attachment #266040 - Flags: review?(enndeakin)
Version: unspecified → Trunk
Comment on attachment 266040 [details] [diff] [review]
patch, icon only

Looks good, but why are you changing the opacity?
Attachment #266040 - Flags: review?(enndeakin) → review+
I'm not changing the opacity, as there's no way how those elements could end up with an opacity below 1. I suppose those rules are left over from older (experimental?) theme versions.
Keywords: checkin-needed
Checking in toolkit/themes/pinstripe/global/browser.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/browser.css,v  <--  browser.css
new revision: 1.23; previous revision: 1.22
done
Checking in toolkit/themes/winstripe/global/browser.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/browser.css,v  <--  browser.css
new revision: 1.34; previous revision: 1.33
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Is this a bug on trunk?

Bug 339964 was checked in.
chrome://global/skin/browser.css doesn't exist.
I feel that this patch is invalid. 
So yes it looks like this patch should have been updated for tabbrowser changes. For some reason the toolkit version of browser.css still exists hence the patch applying cleanly.

Dão, can you confirm and I'll back this out and check in a new patch against the browser skin instead.
Attached patch updated to trunkSplinter Review
Attachment #266345 - Attachment is obsolete: true
Ah, yes. (No need to back out though.)
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Attachment #266040 - Attachment is obsolete: true
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.66; previous revision: 1.65
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.78; previous revision: 1.77
done
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
in-litmus+

http://litmus.mozilla.org/show_test.cgi?id=4699

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007092705 Minefield/3.0a9pre
Status: RESOLVED → VERIFIED
Flags: in-litmus+
Depends on: 402992
You need to log in before you can comment on or make changes to this bug.