Closed
Bug 1020422
Opened 10 years ago
Closed 10 years ago
Clickable area of some in-content preferences too large
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
VERIFIED
FIXED
Firefox 32
People
(Reporter: rittme, Assigned: rittme)
Details
(Whiteboard: p=2 s=33.1 [qa!])
Attachments
(4 files, 1 obsolete file)
The hit area of some in-content preferences is a full block in width but should be limited to just the checkbox and it's label. Some of them are: - Options > General > Tabs preferences - Options > Security - Options > Advanced > Network and Update
Assignee | ||
Comment 1•10 years ago
|
||
This should fix all of the checkboxes and radio buttons with full box width at the in-content options.
Attachment #8435838 -
Flags: review?(mconley)
Flags: needinfo?(mconley)
Comment 2•10 years ago
|
||
Thanks - reviewing now. Just a note that you don't need to both needinfo? and review? me. needinfo? is better if there's a question to be answered. review? does the right thing for what you've been using it for. :)
Assignee: nobody → bernardo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(mconley)
Assignee | ||
Comment 3•10 years ago
|
||
Ok! Sorry about that!
Comment 4•10 years ago
|
||
Comment on attachment 8435838 [details] [diff] [review] rev1 - width of in-content preference checkbox fixed Review of attachment 8435838 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty good! Just a few nits - please see below. Thanks! ::: browser/components/preferences/in-content/advanced.xul @@ +296,5 @@ > + id="allowSmartSize" > + onsyncfrompreference="return gAdvancedPane.readSmartSizeEnabled();" > + label="&overrideSmartCacheSize.label;" > + accesskey="&overrideSmartCacheSize.accesskey;"/> > + <spacer/> A spacer without a size or flex specified doesn't do anything, so this can be removed. ::: browser/components/preferences/in-content/main.xul @@ +192,5 @@ > + <radio id="alwaysAsk" > + value="false" > + label="&alwaysAsk.label;" > + accesskey="&alwaysAsk.accesskey;"/> > + <spacer/> As before, this spacer can be removed. @@ +224,5 @@ > accesskey="&switchToNewTabs.accesskey;" > preference="browser.tabs.loadInBackground"/> > > #ifdef XP_WIN > + <checkbox id="showTabsInTaskbar" label="&showTabsInTaskbar.label;" Hm - the indentation of this block shouldn't have changed. Can you please revert this? ::: browser/components/preferences/in-content/privacy.xul @@ +77,5 @@ > > <!-- Tracking --> > <groupbox id="trackingGroup" data-category="panePrivacy" hidden="true" align="start"> > <caption><label>&tracking.label;</label></caption> > + <radiogroup id="doNotTrackSelection" orient="vertical" align="start" I don't think the doNotTrackSelection radio buttons have this bug. Can you confirm? ::: browser/components/preferences/in-content/security.xul @@ +57,5 @@ > oncommand="gSecurityPane.showAddonExceptions();"/> > </hbox> > > <separator class="thin"/> > + <hbox> Are you sure this outer hbox is necessary? Why not just use the vbox and set align="start" on it? @@ +68,5 @@ > + label="&blockWebForgeries.label;" > + accesskey="&blockWebForgeries.accesskey;" > + preference="browser.safebrowsing.enabled" /> > + </vbox> > + <spacer/> Please remove this spacer. @@ +77,5 @@ > <groupbox id="passwordsGroup" orient="vertical" data-category="paneSecurity" hidden="true"> > <caption><label>&passwords.label;</label></caption> > > <hbox id="savePasswordsBox"> > + <checkbox id="savePasswords" Trailing whitespace.
Attachment #8435838 -
Flags: review?(mconley) → review-
Assignee | ||
Comment 5•10 years ago
|
||
Thank you, Mike. I fixed all of your comments. While I was testing it I found out I forgot to check the non-connected sync screen. It turns out the labels also had a full block width, so I added them to this revision.
Attachment #8435838 -
Attachment is obsolete: true
Attachment #8435977 -
Flags: review?(mconley)
Comment 6•10 years ago
|
||
Comment on attachment 8435977 [details] [diff] [review] rev2 - width of in-content preference checkbox fixed Looks good! Thanks Bernardo!
Attachment #8435977 -
Flags: review?(mconley) → review+
Updated•10 years ago
|
Flags: firefox-backlog+
Keywords: checkin-needed
Comment 7•10 years ago
|
||
Hey Jenn! Would you mind adding this to the current iteration? Thanks!
Flags: needinfo?(jchaulk)
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0947342dbc1b
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0947342dbc1b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Updated•10 years ago
|
Flags: needinfo?(jchaulk)
Whiteboard: S=IT-32C-31A-30B.3
Comment 10•10 years ago
|
||
Hey mconley! Can you give me a size please? And is it qa+ or qa-? Thanks!!
Flags: needinfo?(mconley)
Comment 11•10 years ago
|
||
(In reply to Jenn Chaulk from comment #10) > Hey mconley! Can you give me a size please? And is it qa+ or qa-? Thanks!! Sure. If I had done this, I would have rated it a p=2. This bug was actually fixed by a volunteer contributor (and soon to be intern), so I'm not sure if that makes this a p=0 or not. That stuff's kinda been lost on me. This is a qa+. STR: Visit the following areas of in-content preferences: - General > Tabs preferences - Security - Advanced > Network and Update - Sync (when no FxA account is connected) Notice that for the checkboxes (and some of the links) in the above sections, if you are far to the right of the checkbox label or link, you are still able to toggle the checkbox / click the link. This patch shrinkwraps the click area to just around the checkbox and label / link area. Example GIF (for a different area that was not fixed in this bug, but it visually explains the behaviour we're fixing): https://bug1010402.bugzilla.mozilla.org/attachment.cgi?id=8422607
Flags: needinfo?(mconley)
Updated•10 years ago
|
Whiteboard: S=IT-32C-31A-30B.3 → p=2 s=it-32c-31a-30b.3 [qa+]
Updated•10 years ago
|
Whiteboard: p=2 s=it-32c-31a-30b.3 [qa+] → p=2 s=33.1 [qa+]
Updated•10 years ago
|
QA Contact: camelia.badau
Comment 12•10 years ago
|
||
Verified on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.8.5 and the following mentions should be done: a) for the General, Content, Security, Advanced (General, Data choices, Network) tabs, the clickable area is limited to the checkbox and it's label b) for the Privacy tab ("Use custom settings for history" selected), the clickable area is a full block in width (the clickable area is NOT limited to the checkbox and it's label) -> See screenshot "A.PNG". c) for the Advanced - Update, the clickable area is NOT limited to the checkbox and it's label -> See screenshot "B.PNG" d) for the Sync tab (when FxA account is connected), the clickable area is NOT limited to the checkbox and it's label The actual patch doesn't fixed the issues b), c) and d) mentioned above. Should I file a separate bug for these issues or reopen this one?
Flags: needinfo?(mconley)
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
Comment 15•10 years ago
|
||
Thanks Camelia! Hey Bernardo - interested in taking a look at these follow-ups?
Flags: needinfo?(mconley) → needinfo?(bernardo)
Assignee | ||
Comment 16•10 years ago
|
||
Yes, I would like that! Should I add them to the same patch or create a new one?
Flags: needinfo?(bernardo)
Comment 17•10 years ago
|
||
(In reply to Bernardo Rittmeyer [:rittme] from comment #16) > Yes, I would like that! > Should I add them to the same patch or create a new one? Since this bug is already marked RESOLVED FIXED, I think we'll need a new bug for the new issues (one bug will do), and a new patch. Do you mind filing it and getting started on the patch? I'll happily review!
Assignee | ||
Comment 18•10 years ago
|
||
Sure, I'll fill a new one right away. Thank you!
Comment 19•10 years ago
|
||
Hi Bernardo, have you logged the new bug? Or do you want me to log it?
Assignee | ||
Comment 20•10 years ago
|
||
Hi Camelia, I logged it in bug 1023339.
Comment 21•10 years ago
|
||
Thanks Bernardo!
Status: RESOLVED → VERIFIED
Whiteboard: p=2 s=33.1 [qa+] → p=2 s=33.1 [qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•