Issue 2507 - Adblock Browser Beta Landing Page IE8 bug with inline-block

Related ticket:
https://issues.adblockplus.org/ticket/2507

[saroyanm, 2015-05-12 13:23:55]

Thomas can you please have a look, 
unfortunately the issue also persist in our new CMS as well, so this patch
should fix the issue.

[Thomas Greiner, 2015-05-12 15:06:46]

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
File adblock-browser-head.html (right):

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:38: display: inline-block !important; /* !important to
fix IE 8 bug */
This doesn't resolve the actual issue but merely one of its symptoms.

Generally, you seem to only need this wrapper for making sure that the button is
the only element on that line. Therefore you can simply move all the styles for
`.button-community-wrapper` into the block for `.button-community-wrapper a`
below, set it `display: inline-block` and get rid of this block here entirely.

It's still unclear what the actual issue is but at least you can avoid running
into it this way. Also it doesn't require any special treatment for IE.

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:39: text-align: center;
The block is adjusting to the content inside so the text is already centered
automatically which makes this style redundant.

[saroyanm, 2015-05-12 15:23:47]

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
File adblock-browser-head.html (right):

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:38: display: inline-block !important; /* !important to
fix IE 8 bug */
On 2015/05/12 15:06:47, Thomas Greiner wrote:
> This doesn't resolve the actual issue but merely one of its symptoms.
Exactly, I have no idea why this is happening, It's taking too much time
understanding actual issue. So I would rather in this case fix symptom rather
than the issue itself.
> Generallym , you seem to only need this wrapper for making sure that the
button is
> the only element on that line. Therefore you can simply move all the styles
for
> `.button-community-wrapper` into the block for `.button-community-wrapper a`
> below, set it `display: inline-block` and get rid of this block here entirely.
I would avoid additional changes, because I'm currently manually updating our
CMS. 
Also this change is irrelevant to the issue I think.
> It's still unclear what the actual issue is but at least you can avoid running
> into it this way. Also it doesn't require any special treatment for IE.
Yes exactly.

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:39: text-align: center;
On 2015/05/12 15:06:47, Thomas Greiner wrote:
> The block is adjusting to the content inside so the text is already centered
> automatically which makes this style redundant.

I would rather do this kind of small adjustments after the migration and having
the page live. This looks to be irrelevant to the issue I think and not blocker.

[Thomas Greiner, 2015-05-12 15:41:27]

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
File adblock-browser-head.html (right):

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:38: display: inline-block !important; /* !important to
fix IE 8 bug */
On 2015/05/12 15:23:47, saroyanm wrote:
> On 2015/05/12 15:06:47, Thomas Greiner wrote:
> > This doesn't resolve the actual issue but merely one of its symptoms.
> Exactly, I have no idea why this is happening, It's taking too much time
> understanding actual issue. So I would rather in this case fix symptom rather
> than the issue itself.
> > Generallym , you seem to only need this wrapper for making sure that the
> button is
> > the only element on that line. Therefore you can simply move all the styles
> for
> > `.button-community-wrapper` into the block for `.button-community-wrapper a`
> > below, set it `display: inline-block` and get rid of this block here
entirely.
> I would avoid additional changes, because I'm currently manually updating our
> CMS. 
> Also this change is irrelevant to the issue I think.
> > It's still unclear what the actual issue is but at least you can avoid
running
> > into it this way. Also it doesn't require any special treatment for IE.
> Yes exactly.

The change I suggested is a solution and not an additional change. Therefore
it's not irrelevant.

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:39: text-align: center;
On 2015/05/12 15:23:47, saroyanm wrote:
> On 2015/05/12 15:06:47, Thomas Greiner wrote:
> > The block is adjusting to the content inside so the text is already centered
> > automatically which makes this style redundant.
> 
> I would rather do this kind of small adjustments after the migration and
having
> the page live. This looks to be irrelevant to the issue I think and not
blocker.

It's not a blocker - that's correct - but it can be done as part of the
suggested fix above.

[saroyanm, 2015-05-12 15:44:23]

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
File adblock-browser-head.html (right):

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:38: display: inline-block !important; /* !important to
fix IE 8 bug */
On 2015/05/12 15:41:27, Thomas Greiner wrote:
> On 2015/05/12 15:23:47, saroyanm wrote:
> > On 2015/05/12 15:06:47, Thomas Greiner wrote:
> > > This doesn't resolve the actual issue but merely one of its symptoms.
> > Exactly, I have no idea why this is happening, It's taking too much time
> > understanding actual issue. So I would rather in this case fix symptom
rather
> > than the issue itself.
> > > Generallym , you seem to only need this wrapper for making sure that the
> > button is
> > > the only element on that line. Therefore you can simply move all the
styles
> > for
> > > `.button-community-wrapper` into the block for `.button-community-wrapper
a`
> > > below, set it `display: inline-block` and get rid of this block here
> entirely.
> > I would avoid additional changes, because I'm currently manually updating
our
> > CMS. 
> > Also this change is irrelevant to the issue I think.
> > > It's still unclear what the actual issue is but at least you can avoid
> running
> > > into it this way. Also it doesn't require any special treatment for IE.
> > Yes exactly.
> 
> The change I suggested is a solution and not an additional change. Therefore
> it's not irrelevant.

Okey, sorry just got what you meant.

[saroyanm, 2015-05-12 16:00:02]

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
File adblock-browser-head.html (right):

http://codereview.adblockplus.org/4966660511367168/diff/5629499534213120/adbl...
adblock-browser-head.html:38: display: inline-block !important; /* !important to
fix IE 8 bug */
On 2015/05/12 15:44:23, saroyanm wrote:
> On 2015/05/12 15:41:27, Thomas Greiner wrote:
> > On 2015/05/12 15:23:47, saroyanm wrote:
> > > On 2015/05/12 15:06:47, Thomas Greiner wrote:
> > > > This doesn't resolve the actual issue but merely one of its symptoms.
> > > Exactly, I have no idea why this is happening, It's taking too much time
> > > understanding actual issue. So I would rather in this case fix symptom
> rather
> > > than the issue itself.
> > > > Generallym , you seem to only need this wrapper for making sure that the
> > > button is
> > > > the only element on that line. Therefore you can simply move all the
> styles
> > > for
> > > > `.button-community-wrapper` into the block for
`.button-community-wrapper
> a`
> > > > below, set it `display: inline-block` and get rid of this block here
> > entirely.
> > > I would avoid additional changes, because I'm currently manually updating
> our
> > > CMS. 
> > > Also this change is irrelevant to the issue I think.
> > > > It's still unclear what the actual issue is but at least you can avoid
> > running
> > > > into it this way. Also it doesn't require any special treatment for IE.
> > > Yes exactly.
> > 
> > The change I suggested is a solution and not an additional change. Therefore
> > it's not irrelevant.
> 
> Okey, sorry just got what you meant. 

As discussed internally, the problem still exist with the new approach.

[Thomas Greiner, 2015-05-12 17:16:25]

As discussed internally, the issue is caused by the cssFx library that we're
using throughout adblockplus.org. This issue, however, still persists even with
the latest version of the library and removing this dependency is also not an
option since it might break some pages.

Therefore this fix is the best solution for this problem at the moment.

LGTM but please change the comment to `/* !important to override cssFx styles on
IE8 */`