Closed Bug 1249818 Opened 4 years ago Closed 4 years ago

Top 5 pixels of tabs became completely useless: clicking them neither drag the window, nor select tab.

Categories

(Firefox :: Theme, defect)

47 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 50
Tracking Status
firefox47 --- wontfix
firefox48 --- verified
firefox49 --- verified
firefox50 --- verified

People

(Reporter: arni2033, Assigned: dao)

References

Details

(Keywords: regression, verifyme)

Attachments

(1 file)

>>>   My Info:   Win7_64, Nightly 47, 32bit, ID 20160218030349
STR:
1. Open new window
2. Open new tab in that window
3. Switch it to normal (not maximized) mode
4. Hover mouse over the top 2 pixels of background tab
5. Hold left mouse button
6. Move mouse pointer in any direction

AR:  Nothing

ER:  Either A or B
 A) After Step 5 the background tab should become selected, i.e. fix bug 1200877 already
 B) At least for now (while 1200877 isn't fixed), window should follow the pointer in Step 6

This is regression from bug 1219215 (presumably). Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=be27e36ce427df475b3ff827d8c0258c1c34c5b5&tochange=a67a25a5af7446d8ae9beb453bcfa12be368fcef
I'm pretty sure we'd want to fix this and wontfix bug 1200877, see:

https://dxr.mozilla.org/mozilla-central/rev/af6356a3e8c56036b74ba097395356d9c6e6c5a3/browser/themes/windows/browser.css#2007-2015

and bug 987067 / bug 887397.

It looks like this regression has affected OS X ever since they made the switch away from mozmousehittest, but nobody noticed. Since bug 1219215 it will have affected Windows as well. The cause is that the entire tab element is marked as "no-drag", and we use the clip-path only on the background of the tab. So we should probably tweak the drag region for these items in Firefox's theme code...
OS: Unspecified → All
Hardware: Unspecified → All
(In reply to :Gijs Kruitbosch from comment #1)
Do the following lines look strange to you?
> 1) I'm pretty sure we'd want to fix this and wontfix bug 1200877
> 2) looks like this regression has affected OS X long ago, but NOBODY NOTICED
3) bug 887397 was requested by MAC USERS ONLY
4) You got pretty clear feedback in bug 986942 and 5 other duplicates that it's unwanted on Windows

(3) and (2) mean that they simply wanted a change for change's sake. Therefore (4) means that it's better to let them have their personal regression not affecting at least Windows users.
(In reply to arni2033 from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> Do the following lines look strange to you?
> > 1) I'm pretty sure we'd want to fix this and wontfix bug 1200877
> > 2) looks like this regression has affected OS X long ago, but NOBODY NOTICED
> 3) bug 887397 was requested by MAC USERS ONLY
> 4) You got pretty clear feedback in bug 986942 and 5 other duplicates that
> it's unwanted on Windows
> 
> (3) and (2) mean that they simply wanted a change for change's sake.

You mean that bug 887397 and the 3 dupes that were filed were all from people that were just imagining things, that their problems were somehow not real?

> Therefore (4) means that it's better to let them have their personal
> regression not affecting at least Windows users.

People would be so much more receptive to your arguments if you weren't so adversarial.

Anyway.

Philipp, should we remove the clip path on Windows?
Flags: needinfo?(philipp)
Philipp is out, moving to Stephen - Stephen, can you take a look at comment #3 and let me know your thoughts? Thanks!
Flags: needinfo?(philipp) → needinfo?(shorlander)
I'd like to sum up _facts_ about this whole thing:

1) It started in bug 887397 where _firefox_developers_ (who of course were using MacOS) decided
   to fix their flaw in Australis theme (on MacOS). There're also 2 duplicates reported for MacOS.
2) Bug 887397 was fixed in a very unusual way: instead of increasing margin (or height of empty area)
   it was decided to "sometimes ignore mouse actions applied to background tab"
   Firefox is the only application in my lifetime which does such malicious thing.
3) As a response to bug 887397 (MacOS-only issue, remember?) there were several reports from
   Windows users. It's a real breakage on Windows (let's forget about Australis itself for now).
4) All users in CC list in bug 887397 are familiar to Nightly, except for arminwagner2008, but still
   there're no reports about this issue, and most of those people aren't even in CC list in this bug.
   I conclude that they just wanted that fancy breakage for one moment, and now they forgot about it.
5)(minor) On Windows (if you ever used it), it's really convenient to drag window using area around
   caption buttons (minimize,resize,close).

It'd be a great idea (at least on Windows) to remove the bug implemented in bug 887397 and instead create/improve a special empty area preserved for dragging OR increase draggable area above tabs as said in (2). OR compress background tabs in height (also a bad option), to avoid 'dead areas' and 'magic buttons' occurrences. OR just remove the bug 887397, because there were no such bug on Windows.

// I just noticed that I already left similar comment, but now (4) is way more actual

(In reply to :Gijs Kruitbosch from comment #3)
> You mean that bug 887397 and the 3 dupes that were filed were all from people that were just
> imagining things, that their problems were somehow not real?
That's quite "adversarial" as well, don't you think? Yes, I meant that, and yet I'm right, see (4)
BUT, even if it was real issue, it doesn't mean that the issue must be resolved the way it was "fixed" in bug 887397. Moreover, it doesn't mean that MacOS users can speak for Windows users.
Nobody reported such issue on Windows -> "don't fix it if it ain't broke"
Version: Trunk → 47 Branch
While this is a recent regression in 47, it seems polishy to me. If the end-users happen to click on the top 5 pixels and are unable to switch tabs, they can workaround by using the other areas of the background tab header area and switch tabs. I think it might be too late to fix this in 47 and would not consider it a release blocker.
Can we not wait further for UX input here and just fix the regression? Nothing really changed since this was implemented for Australis, so no strong reason to contest the behavior that we had intentionally implemented.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Dão Gottwald [:dao] from comment #7)
> Can we not wait further for UX input here and just fix the regression?
> Nothing really changed since this was implemented for Australis, so no
> strong reason to contest the behavior that we had intentionally implemented.

How do you intend to fix the regression?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(dao+bmo)
I have no intention, it's your regression ;)

Are you implying that there's no good way to fix this regression? In that case we should just go ahead and remove the clip-path without waiting for UX input, since the current behavior is clearly broken and the worst of all available options.
Flags: needinfo?(dao+bmo)
(In reply to Dão Gottwald [:dao] from comment #9)
> I have no intention, it's your regression ;)
> 
> Are you implying that there's no good way to fix this regression? In that
> case we should just go ahead and remove the clip-path without waiting for UX
> input, since the current behavior is clearly broken and the worst of all
> available options.

Well, removing the clip-path is one option to "fix" the regression instance. I am not sure whether or not there are (immediate/easy/upliftable) others, which is why I asked - I don't know that there's a way to restrict the drag stuff without e.g. applying the clip path to the tabstrip (and even then... not 100% that would fix it) which I'd expect to cause other issues (e.g. with dnd on the tabstrip).

However, I disagree that the current state is necessarily worse than the result of removing the clip path. If we accept the original reason for having the clip path, which was to avoid misclicks/drags on tabs when trying to interact with the titlebar, then the failure mode that results from removing the clip path is worse than having the clip path be dead space. That is, if you try and move a window or otherwise interact with the titlebar and "miss" and hit the tabs instead, "nothing happens" is better than "you detach a tab into a window" or "you change tab selection and lose your context".

As a result I don't think this bug warrants doing anything right now. We can ping Stephen when he's awake and see if we can make progress that way.
I think the decision in bug 887397 was the wrong one.

On OS X you can drag the entire window by using any visible chrome surface and on Windows the window drag area is not very short.

What we currently have due to this bug is dead space. We should make the tab area manipulate the tab and the window area manipulate the window.
Flags: needinfo?(shorlander)
Attached patch patchSplinter Review
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Attachment #8768358 - Flags: review?(gijskruitbosch+bugs)
Component: Tabbed Browser → Theme
Attachment #8768358 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/4eedf5c4e7e2
Remove clip-path from background tabs. r=gijs
https://hg.mozilla.org/mozilla-central/rev/4eedf5c4e7e2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Keywords: verifyme
See Also: 1200877
Duplicate of this bug: 986942
Comment on attachment 8768358 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1219215
[User impact if declined]: see comment 0
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: simple code removal, low risk
[String/UUID change made/needed]: none
Attachment #8768358 - Flags: approval-mozilla-beta?
Attachment #8768358 - Flags: approval-mozilla-aurora?
Comment on attachment 8768358 [details] [diff] [review]
patch

Review of attachment 8768358 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes a regression. Take it in beta 7 and aurora.
Attachment #8768358 - Flags: approval-mozilla-beta?
Attachment #8768358 - Flags: approval-mozilla-beta+
Attachment #8768358 - Flags: approval-mozilla-aurora?
Attachment #8768358 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
I managed to reproduce this bug on Nightly 47.0a1 (2016-02-19).
I also investigated it using 
- Windows 10 x64
- Ubuntu 14.04 x86
- Mac OS X 10.11.1

There are some problems with latest Nightly 50.0a1 (2016-07-10) build on Mac - there is a narrow area
for which two actions are overlapping: window dragging and tab dragging (see the screencast https://drive.google.com/file/d/0B0nYKG6PRiCcVFdNMEh2a21NVnc/view?usp=sharing).
Also, it seems that on Windows and Mac the fix didn't land yet for Aurora and Beta. I will continue the verification process when the fix will land.
(In reply to Iulia Cristescu, QA [:IuliaC] from comment #19)
> on Mac - there is a narrow area for which two actions are overlapping
Could you please check if it was reproducible before bug 887397, e.g. on Nightly from 2014-03-06 ?
Flags: needinfo?(iulia.cristescu)
(In reply to arni2033 from comment #20)
> (In reply to Iulia Cristescu, QA [:IuliaC] from comment #19)
> > on Mac - there is a narrow area for which two actions are overlapping
> Could you please check if it was reproducible before bug 887397, e.g. on
> Nightly from 2014-03-06 ?

I tested this on Nightly 2014-03-06 and also on Nightly 2014-03-08 and I couldn't reproduce the overlapping. Besides this, this problem is encountered on Nightly 2016-07-06 (before Bug 1249818 fix landed to mozilla-central). I will further investigate this as soon as possible.
Flags: needinfo?(iulia.cristescu)
I managed to verify this bug on 
- latest Nightly 50.0a1 (2016-07-13)
- latest Aurora 49.0a2 (2016-07-13)
- 48.0b7 build1 (20160711002726)
using 
- Windows 10 x64
- Ubuntu 14.04 x86
- Mac OS X 10.11
The fix landed and it works as expected. I didn't notice any misbehaviour, except the issue mentioned in comment 19, but it seems that this is an old regression and so, I logged Bug 1286545 for it. 
I will set the corresponding flags.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.