Two issues here:

1 - the username is indented - is this meant to be like this? looks like a bug.
2 - the textarea scrollbar is cut off, about 50% of it is not displayed.
bartik-comment-form-IE7.png

Comments

Jeff Burnz’s picture

I could get to this soon, but if someone else wants to jump in a roll a patch ;)

nevergone’s picture

StatusFileSize
new341 bytes

textarea patch

Jeff Burnz’s picture

+++ b/themes/bartik/css/ie.css
@@ -24,3 +24,7 @@ fieldset legend {
+  width: 680px;

This can't be a fixed width, the textarea has to be fluid to account for different sidebar configurations.

I've seen this in a few places in Bartik so we must have a bug somewhere in the CSS that's causing this general problem.

Powered by Dreditor.

reglogge’s picture

Status: Active » Needs review
StatusFileSize
new1.07 KB

It's the left and right padding of 4px each in the textarea. Set these to 0px for IE7 and below in ie.css and the scrollbar is there.

The seemingly indented username stems from both the link (a.username) and the input element below being styles with width: 75%, float: right and padding: 4px. The padding doesn't take in IE6 & 7 (and other real browsers as well) for the link since its not rendered as a block. Adding margin-right: 8px to the link fixes this too.

Patch attached.

Jeff Burnz’s picture

Title: Comment form needs work for IE7 » Comment form needs work for IE6 and 7
StatusFileSize
new8 KB

This looks good - I've checked in IE6, 7, 8, FF, Chrome - looks good both for logged in and anon comments.

I realise the zero horizontal padding is not the prettiest fix for IE6/7, but is the easiest fix at the moment. I think we have to run with it, because the alternative is either:

1 - hacky percentage overrides for both the width of the textarea and the padding - not liking this, its fragile
2 - dynamically setting the width (in pixels) based on the active sidebars - not a big fan of this either because it will take a lot of testing

I found another issue with the comment form labels in IE6 - can we try and fix this as well? If not then we can RTBC this now and get it in, then go for the label fix for IE6 in another issue - I leave it up to you, we're not supposed to creep scope like this but since we are so few managing many separate issues is really hard (read - I have re-roll so many patches every time one tiny patch gets committed...).

reglogge’s picture

StatusFileSize
new1.54 KB

I feel your pain regarding rerolling patches. The fix for the IE6 issue in #5 is easy enough: Just add

#comment-form div {
  clear: left;
}

to ie6.css.

Patch attached. Note however :-( that this patch does not apply together with the patches in #660614: Remove #block-system-main dependency, fix font sizes, remove crufty CSS. So one of the two should go in first and then let's do a reroll of the other one. It's probably better to first get this one in.

Jeff Burnz’s picture

Status: Needs review » Reviewed & tested by the community

OK, lets go for it, looks good, tested and approved. I can't see another way out of this without some serious hacking on textarea widths, IE8+ support box-sizing so these old browsers will just have to have a no-padding experience.

reglogge’s picture

Status: Reviewed & tested by the community » Postponed

Postponing this one until #660614: Remove #block-system-main dependency, fix font sizes, remove crufty CSS lands. They don't apply both and the other issue is more important.

Jeff Burnz’s picture

Status: Postponed » Needs review

#6: 876960-6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 876960-6.patch, failed testing.

reglogge’s picture

Status: Needs work » Needs review
StatusFileSize
new402 bytes

The new patch should apply now. It contains only the fix for the textarea width (and disappearing scrollbars) in IE7 and below since the other fixes don't seem to be necessary any more.

reglogge’s picture

Version: 7.x-dev » 8.x-dev
StatusFileSize
new422 bytes

Rerolled the patch against current HEAD. This can also be backported.

droplet’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Needs work
Issue tags: +Novice, +Needs manual testing
reglogge’s picture

@droplet: any particular reason why you set this back to 7.x-dev?

droplet’s picture

reglogge’s picture

Hallelujah!

yashadev’s picture

Status: Needs work » Needs review
star-szr’s picture

bryandrew19’s picture

Issue tags: +Novice, +Needs manual testing

#2: 876960-1.patch queued for re-testing.

aloyr’s picture

will try to reroll this patch
drupalcon portland

parthipanramesh’s picture

Sorry but the path has changed. It's themes/bartik/... now.
error: core/themes/bartik/css/ie.css: No such file or directory

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.