Closed Bug 871158 Opened 9 years ago Closed 6 years ago

Windows: Missing borders around the toolbox and customize area in customization mode

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: Alopepeo, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P4+] [qx])

Attachments

(1 file, 3 obsolete files)

Attached image Missing borders when maximized (obsolete) —
1. Maximize your Firefox window 
2. Enter customization mode

Result:

There are no borders at both sides of the nav-bar and at the bottom of the customization palette.

Expected result:

The appearance of the customization mode should be the same on maximized and restored windows.
Blocks: 770135
Summary: There is no right-left borders in the nav bar and bottom border in the customization palette when you're in customization mode and the window is maximized → Missing borders when you're in customization mode and the window is maximized
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86_64 → All
I think the changes for bug 870574 might be useful for Windows and Linux to get the same shadow/border when in customization mode.
No longer blocks: 770135
Whiteboard: [Australis:M7]
Assignee: nobody → gijskruitbosch+bugs
(In reply to Alejandro Rodriguez [:Alopepeo] from comment #2)
> Created attachment 748433 [details]
> Missing borders when maximized
> 
> 1. Maximize your Firefox window 
> 2. Enter customization mode
> 
> Result:
> 
> There are no borders at both sides of the nav-bar and at the bottom of the
> customization palette.
> 
> Expected result:
> 
> The appearance of the customization mode should be the same on maximized and
> restored windows.

FWIW, as far as I can tell there are currently no borders in restored windows, either. :-\
Attached patch WIP (obsolete) — Splinter Review
This is where I'm at at the moment. It sort of works, but I'm having issues with the navbar's top border / tabbar's bottom border. It needs to have a radius together with the side borders of the navbar, but I can't seem to get this to work properly, and I also can't figure out where the bottom border on the tab bar is actually coming from and/or how I'd workaround the bad effects it's creating here.
Attachment #764762 - Flags: feedback?(mnoorenberghe+bmo)
Attached image Screenshot of what I'm seeing (obsolete) —
Apologies for slight blurriness, screenshot was over VNC from a hidpi mac. Doesn't detract from the visibility of the issue here though.

So, Matt, I couldn't figure out where the tab's border comes from, on Windows. Any clues? :-)
Removing the items from M7 that do not block landing on m-c.
Whiteboard: [Australis:M7] → [Australis:M?]
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P4]
Comment on attachment 764762 [details] [diff] [review]
WIP

Matt and I talked about this, it's a gradient. Clearing feedback request for now, and marking this obsolete as it won't do, obvs.
Attachment #764762 - Attachment is obsolete: true
Attachment #764762 - Flags: feedback?(mnoorenberghe+bmo)
Attached patch Patch v1Splinter Review
OK, this works well in Aero Basic, Aero Glass, and Win7 Classic. Testing XP in a bit.
Attachment #748433 - Attachment is obsolete: true
Attachment #764765 - Attachment is obsolete: true
Attachment #773217 - Flags: review?(mnoorenberghe+bmo)
(In reply to :Gijs Kruitbosch from comment #7)
> Created attachment 773217 [details] [diff] [review]
> Patch v1
> 
> OK, this works well in Aero Basic, Aero Glass, and Win7 Classic. Testing XP
> in a bit.

XP Seems to look good with this as well (save other issues with how far the titlebar extends... but that has nothing to do with this bug).
Ugh. That code looks like it's guaranteed to break whenever we touch the border styling for when you're not customizing. Can we drop the odd grid around the window while customizing? It causes a whole class of bugs such as bug 885069, bug 883145, bug 885066 / bug 870602 and it seems questionable whether it improved usability even if it worked as intended (e.g. it reduces the space available in toolbars, which isn't at all what you want while customizing).
Comment on attachment 773217 [details] [diff] [review]
Patch v1

As I've communicated in-person to multiple people, I agree with Dão that this custom window padding for customization mode seemingly adds more costs in maintainability/complexity than the benefit it's supposed to achieve. There is also the performance of the animation (bug 873060) which still needs more work otherwise the browser feels janky upon entering/exiting customization mode (I'm testing on an XP VM with the latest UX Nightly).

I discussed this with shorlander who was fine to get rid of it but he deferred me to Madhava who wants to keep it if possible.  We discussed the benefits it's intended to achieve but I think I'm forgetting some (there was some blurriness between some IIRC). From memory here are some 
1) A clear indication that the user is in a special mode to help explain why things work differently.
2) It's somewhat whimsical with the animation and something you can show off to friends. This gives some kind of attachment and can be somewhat viral.

This patch is too fragile as Dão says. Have you looked into changing the rules/rulesets used for adding the borders in the first place which would reduce duplication? e.g. add a rule to some rulesets that cause the border to apply in customization mode when it wouldn't normally. Let me know if this isn't any cleaner.
Attachment #773217 - Flags: review?(mnoorenberghe+bmo) → review-
Status: NEW → ASSIGNED
Summary: Missing borders when you're in customization mode and the window is maximized → Windows: missing borders around the toolbox and customize area in customization mode
Duplicate of this bug: 885050
Whiteboard: [Australis:M?][Australis:P4] → [Australis:M?][Australis:P4] [defect] p=0
This also affects at least OS X as well.
Summary: Windows: missing borders around the toolbox and customize area in customization mode → Missing borders around the toolbox and customize area in customization mode
(In reply to Mike Conley (:mconley) from comment #12)
> This also affects at least OS X as well.

This confuses me. The OS X bits were implemented in bug 870574. Do we know when they regressed?
Flags: needinfo?(mconley)
I think I'm totally wrong on this bug - I thought this bug was reporting missing dotted outlines around customization areas, but a re-read makes it clear that it's about borders around the toolbar / customization elements themselves.

So please ignore comment 12 for now. :)
Flags: needinfo?(mconley)
Summary: Missing borders around the toolbox and customize area in customization mode → Windows: Missing borders around the toolbox and customize area in customization mode
No longer blocks: fxdesktopbacklog
Whiteboard: [Australis:M?][Australis:P4] [defect] p=0 → [Australis:M?][Australis:P4]
(In reply to Mike Conley (:mconley) from comment #14)
> I think I'm totally wrong on this bug - I thought this bug was reporting
> missing dotted outlines around customization areas, but a re-read makes it
> clear that it's about borders around the toolbar / customization elements
> themselves.
> 
> So please ignore comment 12 for now. :)

Is about the borders (https://bug871158.bugzilla.mozilla.org/attachment.cgi?id=748433). It's in maximized mode only.
philipp says this is highly visible as a problem on Windows when full-screen.
Whiteboard: [Australis:M?][Australis:P4] → [Australis:P4+]
(In reply to Justin Dolske [:Dolske] from comment #17)
> philipp says this is highly visible as a problem on Windows when full-screen.

Can we add this to the backlog for the upcoming iteration?

Unassigning because I'm not currently working on this.
Assignee: gijskruitbosch+bugs → nobody
Flags: needinfo?(dolske)
Flags: firefox-backlog+
Flags: needinfo?(dolske) → needinfo?(gavin.sharp)
See bug 941244 comment 9 - same applies here.
Flags: needinfo?(gavin.sharp)
Whiteboard: [Australis:P4+] → [Australis:P4+] [qx]
checked in a current nightly on phlsa's VM, and this appears to have been fixed at some point. (Comparing to screenshot in comment 16)
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.