Make WordPress Core

Opened 6 years ago

Closed 13 months ago

#46771 closed defect (bug) (fixed)

Twenty Eleven: Negative values for padding

Reported by: malae's profile Malae Owned by: karmatosed's profile karmatosed
Milestone: 6.6 Priority: normal
Severity: minor Version: 4.9.8
Component: Bundled Theme Keywords: has-patch 2nd-opinion commit
Focuses: css Cc:

Description

Line 2799
#ie7 article.intro Value Error : padding-left -7.6% negative values are not allowed : -7.6%

Line 2800
#ie7 article.intro Value Error : padding-right -7.6% negative values are not allowed : -7.6%

Attachments (3)

46771.diff (475 bytes) - added by mukesh27 6 years ago.
Patch that remove left and right padding in IE
46771.1.diff (484 bytes) - added by sabernhardt 4 years ago.
positive padding of 7.6%
46771.2.diff (439 bytes) - added by sabernhardt 4 years ago.
remove the two faulty lines

Download all attachments as: .zip

Change History (15)

@mukesh27
6 years ago

Patch that remove left and right padding in IE

#1 @mukesh27
6 years ago

  • Keywords has-patch added; needs-patch removed

#2 @SergeyBiryukov
6 years ago

  • Component changed from Themes to Bundled Theme
  • Summary changed from Twentyeleven theme style.css to Twenty Eleven: Negative values for padding

#3 @sabernhardt
4 years ago

  • Focuses css added
  • Keywords 2nd-opinion added

Thanks for the report and patch!

Because older Internet Explorer versions are hardly used and no longer officially supported, this would be a candidate for closing as won't fix. However, I think the theme's authors intended to have positive padding of 7.6% on the left and right sides.

Other possibilities:

  1. Remove the lines that set padding for IE7 (so it falls back to 8.9%)
  2. Remove all IE7 styles

@sabernhardt
4 years ago

positive padding of 7.6%

@sabernhardt
4 years ago

remove the two faulty lines

#4 @sabernhardt
4 years ago

Correcting the values this late seems quite absurd. So I uploaded another patch for removing those lines.

If we want to remove more, that would belong in another ticket (to address older IE conditional CSS in Twenty Ten to Twenty Seventeen).

#5 @sabernhardt
2 years ago

This may be removed as part of #56699.

#6 @poena
18 months ago

This has not been removed, shall we move forward with patch 46771.2.diff?

#7 @karmatosed
14 months ago

  • Keywords commit added

I would agree with everyone that removing those lines makes sense. I am therefore going to agree that going ahead with the diff to do so makes sense. I will add the commit keyword to take it through.

#8 @karmatosed
14 months ago

  • Keywords commit removed

I'm going to go back on my own decision here, apologies. I read through https://core.trac.wordpress.org/ticket/56699. It feels to me like if there has been work done to remove IE specific lines entirely, that removing this all would be the better move over just the negative pieces.

I am going to include @desrosj in this conversation as the originator of the ticket and work on IE specific. On reflection I do think that committing IE 7 changes, even going back over a past bug feels not the best when we perhaps could just remove them. I am happy to be wrong but it did make me pause myself on committing this, apologies there.

#9 @sabernhardt
14 months ago

  • Milestone changed from Awaiting Review to 6.6

See also #58836

This ticket was mentioned in Slack in #core-themes by karmatosed. View the logs.


13 months ago

#11 @karmatosed
13 months ago

  • Keywords commit added
  • Owner set to karmatosed
  • Status changed from new to assigned

Thank you everyone, this came up in the scrub today and was decided to commit.

#12 @karmatosed
13 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58005:

Twenty Eleven: Removes Negative values for padding.

This fixes the issue where negative values aren't allows in older versions of IE. There is more discussion in another ticket on a wider approach, but for now the decision to commit this was made as an interim solution.

Props mukesh27, SergeyBiryukov, sabernhardt, poena.
Fixes #46771.

Note: See TracTickets for help on using tickets.