labadserver / Adplayer

Adplayer reference implementation
https://github.com/labadserver/Adplayer/wiki
Other
28 stars 11 forks source link

More CSS fixes #109

Closed ingridgraefen closed 12 years ago

ingridgraefen commented 12 years ago

I have tested the display of the privacy button and panel on the homes of 44 sites with various layouts. In my test scenario the button is a direct child of the body (containing general info for the page).

My test is done with js file from version 1.5.2, but with the recent style.css downloaded from github yesterday night (2012-06-14 23:00).

The tests showed some problems.


I have already fixed all the problems listed below in a local version of style.css.

I will provide this file later today.


More problems might show up when the button is injected in ad containers where more css definitions of a site can interfere.

Here are the general problems I found and the remedies/rules that we should use for all our css definitions.


(1) CSS must work in IE quirksmode

Issue with adserver iframe tags and with a minor number of sites in quirksmode.

Some adservers like old DFP create an iframe document without doctype and deliver the ad in this iframe. The privacy button is then in quirksmode. See (2) for the offset of the button mentioned by Oliver Wagner.

(2) Never define multiple classes in the css file (like .abc.xyc)

They do not work in IE in quirksmode and IE < 7. Only the last definition in such a chain will be applied.

The current multiple class definitions of: .adp-admarker-text.adp-top-right {right: 15px;} .adp-admarker-text.adp-top-left {left: 15px;}

are the reason for the offset of the button in some situations. IE quirksmode discards the first class name, thus clobbers the 0 value of .adp-top-right or .adp-top-left.

(3) Do not use :hover pseudo class on non-link elements

In IE quirksmode :hover is only applied on links. In the quirksmode scenario, the mouseover info on the button is not displayed because this does not work: .adp-admarker:hover .adp-admarker-text {display: block;} * This remains a todo after updating style.css *

This is relevant for adserver iframe tags, see (1). The display of the mouseover text must be handled via javascript mouseover/mouseout events.

(4) Never use the unit em for line-height, use unit-less line-heights or px

Inheritance of em works like this: em calculates the line-height once and only once depending on the present font-size. Child elements inherit this static line-height regardless of their font-size. This is almost always unwanted.

Inheritance of unit-less line-heights works as expected, i.e. dynamically depending on the applied font-size.

This is the reason for the initially buggy line-heights in the panel. Currently, this has been remedied by setting em line-heights on many elements.

Instead of this, a unit-less line-height of 1.3 should be defined on the outer element.

See: http://meyerweb.com/eric/thoughts/2006/02/08/unitless-line-heights/

(5) Increase the specificity of critical definitions by adding parent selectors to the chain, like

div.adp-wrapper div.adp-panel div.adp-wrapper div.adp-admarker div.adp-wrapper div.adp-admarker .adp-admarker-text

This is an issue with some sites defining general properties depending on the id of the body tag, or some such.

Example

mybody div {margin: 0px; padding: 0px;}

This wins over the current panel margins/paddings.

(6) Define properties in as few places as possible and force inheritance by high specificity.

Currently, properties like font-family, line-height, font-size are defined on several selectors.

(7) Define the padding between the outer white container and the inner header, info, footer containers by padding on the panel. Remove the horizontal padding/margin from the inner containers. This makes changes easier.

(8) Increase the horizontal and bottom padding for the panel from 5px to 8px. 5px looks a little too tight.

(9) Specify font-weight for links explicitly

Currently, the links are normal on most sites, and bold on some.

ingridgraefen commented 12 years ago

Here are some links to demonstrate display with style.css from yesterday night and the changes. The first link (yshowroom=github) is style.css from yesterday. The second link (yshowroom=ingrid) is my changed file.

The button appears in the top right browser corner with the load event.

(a) bold links http://www.prosieben.de/?showroom=x&yshowroom=oba&yshowroom=github http://www.prosieben.de/?showroom=x&yshowroom=oba&yshowroom=ingrid

(b) margin and padding are clobbered by site css http://www.prosiebengames.de/?showroom=x&yshowroom=oba&yshowroom=github http://www.prosiebengames.de/?showroom=x&yshowroom=oba&yshowroom=ingrid

(c) Demo for the IE quirksmode problem. Use either (a) or (b) and force quirksmode with IE Developer Tools