Closed Bug 1032790 Opened 6 years ago Closed 5 years ago
In-content preferences: Close button missing from the Exceptions sub-dialog on Mac OSX
71.66 KB, image/png
8.70 KB, image/png
24.18 KB, image/png
2.00 KB, patch
|Details | Diff | Splinter Review|
This bug was filled based on the discussion from bug 1021618 comment 34. Reproducible on the latest Nightly (BuildID: 20140630030202) Mozilla/5.0 (Machintos; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0 Steps to reproduce: 1. Launch Firefox. 2. Open about:preferences. 3. Open any Exceptions sub-dialog. Actual results: The Close button is missing from all the Exceptions sub-dialogs on Mac OSX. Expected results: The Close button should be added to the Exceptions sub-dialogs on Mac OSX?
I don't know who from UX can answer so I'm needinfoing :Boriss. Jennifer, what do you think, should the close button also be shown on the Exceptions in-content sub-dialogs? Best you are looking on Content/Pop-ups - Exceptions... as the other are not yet implemented as in-content. But you can use the other as a reference for the normal dialogs.
(In reply to Richard Marti (:Paenglab) from comment #1) > I don't know who from UX can answer so I'm needinfoing :Boriss. > > Jennifer, what do you think, should the close button also be shown on the > Exceptions in-content sub-dialogs? Best you are looking on Content/Pop-ups - > Exceptions... as the other are not yet implemented as in-content. But you > can use the other as a reference for the normal dialogs. The close button on the dialogs in Content can be a model for other dialogs. The kerning and leading and margins in these dialogs need some improvement, but the grey close x (attached) is fine to apply to all in-content sub-dialogs.
I think Jennifer had only the close button on top right in view. But I think the "Close" button on bottom left should be shown also on OS X as only this dialogs have only the ability to close the dialog by the [X] button. All other dialogs have either a "Close" or "Cancel" button.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Comment on attachment 8469982 [details] [diff] [review] dialogCloseButton.patch This is intended behaviour to match the platform and I think your change to permissions.xul will affect when it's opened outside of in-content preferences. I think UX should be consulted to understand if/how we want to diverge from platform styling and when we want to do that (only for in-content preferences or for other dialogs too). The alternative is to reverse this fix and remove the buttons from other dialogs on OS X because instant-apply is on for OS X and it matches the OS better.
Attachment #8469982 - Flags: feedback-
See comment 4
Comment on attachment 8469982 [details] [diff] [review] dialogCloseButton.patch Removing r? until Philipp's feedback.
I'm not sure I fully grasp the specific problem, so here's a more general approach :) - In-content dialogs shouldn't auto-apply (the dialogs are modals, and modals usually don't auto-apply) - Every in-content dialog should have a »Cancel/Done« button combo (there might be better labels for these) - Every in-content dialog should have a close button [x] that does the same as the cancel button - These rules apply to all operating systems - None of these rules influences dialogs that are not in-content (we should look at these separately) Sevaan, do you think this makes sense?
Flags: needinfo?(philipp) → needinfo?(sfranks)
OK, if we don't want the subdialogs to instant-apply on any OS then that's the opposite of the direction we were headed with in-content preferences (as we were going to turn it on for all OSs instead of just OS X for in-content subdialogs). It sounds like in-content preferences which aren't modal (i.e. the main panes) should instant-apply but the subdialogs shown from it shouldn't? Dialogs outside of in-content preferences should continue to follow OS behaviour (i.e. instantApply on OS X only). I think the proposal from comment 7 makes sense. I'll wait to see what Sevaan says.
The Exceptions dialog are always a kind of instant-apply. They are adding/removing sites from lists and there is no rollback when you click the close button.
On the dialogs I change there is a "Close" not a "Cancel" button and also no meaning of restoring. On Linux and Windows they are already visible.
Hey :paenglab, I don't have VMWare installed, so I can't do this...would you mind posted a screenshot of an Exception window for Mac and the same one for Windows just so I can see both? Sorry for the trouble and thanks!
See Also: → 421282
I add a needinfo for sevaan to not forget this bug.
I /did/ forget! Sorry, and thanks for the prompt! Yes, the close button should be added to the dialog. All other OSs have it, so we should include it for the sake of consistency.
Okay, asking again for review with updated to tip patch after sevaan's positive feedback.
Comment on attachment 8492176 [details] [diff] [review] dialogCloseButton.patch Review of attachment 8492176 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. Comment #7 mentions a lot of requirements, but not all of those need to be fixed in this bug.
Attachment #8492176 - Flags: review?(MattN+bmo) → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Flags: qe-verify? → qe-verify+
QA Contact: camelia.badau
Verified fixed on Mac OSX 10.9.5 using latest Nightly 37.0a1 (buildID: 20141214030205).
Depends on: 1123128
You need to log in before you can comment on or make changes to this bug.