Opened 3 years ago

Closed 3 years ago

#4656 closed defect (fixed)

YouTube video on ABP homepage positioned incorrectly in Nightly

Reported by: Ross Assignee: saroyanm
Priority: P2 Milestone:
Module: Websites Keywords:
Cc: saroyanm Blocked By:
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Ross Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29366870/

Description (last modified by Ross)

Environment

Firefox Nightly 53.0a1 (2017-11-20)

How to reproduce

  1. Visit https://adblockplus.org in Firefox Nightly (53).

Observed behaviour

In Nightly, the YouTube video on the homepage is pushed below the AdblockBrowser banner in the layout, even if the browser/page space is maximised (see Nightly.jpg).

Expected behaviour

The YouTube video to be above the banner to the left of the Install for X button as it is in FF 38/49 (see 49.jpg).

Attachments (2)

49.jpg (262.7 KB) - added by Ross 3 years ago.
Nightly.jpg (234.2 KB) - added by Ross 3 years ago.

Download all attachments as: .zip

Change History (14)

Changed 3 years ago by Ross

Changed 3 years ago by Ross

comment:1 Changed 3 years ago by Ross

  • Description modified (diff)

comment:2 Changed 3 years ago by Ross

  • Description modified (diff)

comment:3 Changed 3 years ago by juliandoucette

Interesting!

@saroyanm applyng vertical-align: top; to the iframe seems to fix this issue. I'm not sure if this is our bug or theirs (mozilla). I reported it here: https://bugzilla.mozilla.org/show_bug.cgi?id=1320660

comment:4 Changed 3 years ago by juliandoucette

  • Cc saroyanm added

comment:5 Changed 3 years ago by juliandoucette

  • Priority changed from Unknown to P1
  • Ready set

Looks like an issue on our end. See https://bugzilla.mozilla.org/show_bug.cgi?id=1320660 for more details.

Julian, thank you for reporting this issue. Alice0775, the reduced html is helpful! Thank you.

First of all, I cannot reproduce this issue (either the original adblock site or the reduced html in comment 4) on my MBP retina display, but I could see the video being push down by using an external low resolution display, so I guess it's somewhat related to font and screen resolution.

Anyway, this is exact the issue I fixed in Bug 1291110. Let me explain.

Both <section id="main"> and <div id="message-wrapper"> are float: right. Since inline elements cannot intersect floats, it's expected that the <iframe id="video"> is pushed down if it is too tall to fit into the space to the left of <section id="main">.

On the external display, the video element seems just 1px too tall to fit the space. I could fix this issue by either
a) increase the height of <section id="main"> from 307px to 308px, or
b) have <iframe id="video"> go before <div id="message-wrapper">.

If I shrink the height of <section id="main"> to about 302px, I could see the same behavior on Chrome 54 and Safari 10.0.1. So I feel it's possible some of the users might experience the same issue on other browsers with different font or screen resolution. So it'll be the best if the adblock site could be fixed.

BTW, on the old Firefox (before bug 1291110 was fixed), the video element will intersect the float <div id="message-wrapper"> even if it's too tall, so this issue doesn't happen.

comment:6 Changed 3 years ago by juliandoucette

  • Priority changed from P1 to P2

comment:7 Changed 3 years ago by juliandoucette

I set Ready and priority because this also occurs in the latest Firefox.

comment:8 Changed 3 years ago by juliandoucette

Note: There have been more developments on https://bugzilla.mozilla.org/show_bug.cgi?id=1320660 since my last comment.

I still think we should fix this issue on our end regardless.

comment:9 Changed 3 years ago by saroyanm

  • Owner set to saroyanm

comment:10 Changed 3 years ago by saroyanm

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:11 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4656 - fixed wrongly positioned iframe in FF nightly

comment:12 Changed 3 years ago by saroyanm

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.