Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#11422 closed (duplicate)

Broken html in CheckboxSelectMultiple widget (with fix)

Reported by: Jerome Leclanche Owned by: bmihelac
Component: Forms Version: master
Severity: Keywords:
Cc: adys.wh@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

The html in CheckboxSelectMultiple widget is broken, under the format <li><label%s>%s %s</label></li>. This itches Gecko (noticeable with firebug), and completely breaks webkit.

Attached patch, and before/after screenshot.

Attachments (4)

checkboxselectmultiple.patch (721 bytes) - added by Jerome Leclanche 7 years ago.
Patch
beforeafter.png (28.5 KB) - added by Jerome Leclanche 7 years ago.
Before / After (Webkit)
Screen shot 2010-02-28 at 11.12.18 AM.png (11.1 KB) - added by Idan Gazit 7 years ago.
Rendering in RTL locales
11422.diff (418 bytes) - added by bmihelac 7 years ago.

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by Jerome Leclanche

Patch

Changed 7 years ago by Jerome Leclanche

Attachment: beforeafter.png added

Before / After (Webkit)

comment:1 Changed 7 years ago by Jerome Leclanche

Cc: adys.wh@… added

comment:2 Changed 7 years ago by Jerome Leclanche

Why hasn't this been committed yet?

comment:3 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 Changed 7 years ago by jkocherhans

This patch looks fine, but I think the checkboxes should be on the left, not the right. The css is the culprit here. Anyone want to fix it and upload a new patch?

Changed 7 years ago by Idan Gazit

Rendering in RTL locales

comment:5 Changed 7 years ago by Idan Gazit

Existing patch also breaks down in RTL locales, see attached screenie (webkit)

comment:6 Changed 7 years ago by Russell Keith-Magee

See #13131 for a proposed fix for the RTL issue.

comment:7 Changed 7 years ago by Patryk Zawadzki

The HTML is not broken. What is broken are CSS rules applied to the widget in the admin app. I have proposed a fix in #13131 (which was closed as a duplicate of this one).

comment:8 Changed 7 years ago by bmihelac

Owner: changed from Jerome Leclanche to bmihelac

I agree with patrys, CSS for admin is problematic and not HTML.
In forms.css labels are floated to the left which is not ok good for multiple checkboxes because it makes whole li element floated.
The simplest solution is to remove floating form label if it is inside li element.
I am attaching patch for this.

Changed 7 years ago by bmihelac

Attachment: 11422.diff added

comment:9 Changed 7 years ago by bmihelac

I just found that this issue is duplicate of #12260

comment:10 Changed 7 years ago by Patryk Zawadzki

This one is was reported earlier so I'd say #12260 is a duplicate of this one.

comment:11 in reply to:  10 Changed 7 years ago by bmihelac

Replying to patrys:

This one is was reported earlier so I'd say #12260 is a duplicate of this one.

#12260 better describes the actual problem and looking at it now, it also reset label to display and width settings.
I think it is safe to close this one as invalid.

comment:12 Changed 6 years ago by stemmetje

Resolution: duplicate
Status: newclosed

This issue was fixed under ticket #12260, so I'm marking this ticket as a duplicate.

comment:13 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top