1230050 - "Rating is available…" and "This feature is not available right now.…" text appear for Reader Mode on YouTube videos

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect)

Description

[Matthew N. [:MattN]]

STR:
1) Load the given URL

Expected result:
The video title and description appear in reader mode

Actual result:
The video title and description appear in reader mode but there is some additional text that isn't normally visible to users outside of Reader Mode:

> Vienna-Billy Joel (Lyrics in Description)
> 
> Rating is available when the video has been rented.
> 
> This feature is not available right now. Please try again later.
> 
> Uploaded on Dec 28, 2009
> 
> Gente, dejo esta canción, una de mis tantas favoritas :)
> Lyrics:

"Rating is available when the video has been rented." and "This feature is not available right now. Please try again later." should not appear IMO.

These come from the following markup:

> <div id="watch-action-panels" class="watch-action-panels yt-uix-button-panel hid yt-card yt-card-has-padding">
>   …
>  <div id="action-panel-rental-required" class="action-panel-content hid">
>     <div id="watch-actions-rental-required">
>       <strong>Rating is available when the video has been rented.</strong>
>     </div>
>   </div>
> 
>   <div id="action-panel-error" class="action-panel-content hid">
>     <div class="action-panel-error">
>       This feature is not available right now. Please try again later.
>     </div>
>   </div>
> 
>     <button class="yt-uix-button yt-uix-button-size-default yt-uix-button-default yt-uix-button-empty yt-uix-button-has-icon no-icon-markup yt-uix-button-opacity yt-uix-close" type="button" onclick=";return false;" id="action-panel-dismiss" aria-label="Close" data-close-parent-id="watch8-action-panels"></button>
> </div>

#action-panel-rental-required and #action-panel-error have `display:none` via the "hid" class so we could maybe use that to realize we shouldn't show it.

> .hid {
>   display: none;
> }

Comment 1

[:Gijs (out for now; he/him)]

(In reply to Matthew N. [:MattN] from comment #0)
> STR:
> 1) Load the given URL

> #action-panel-rental-required and #action-panel-error have `display:none`
> via the "hid" class so we could maybe use that to realize we shouldn't show
> it.

We have no way of knowing that if we directly load the URI, because we XHR the article without rendering it, and so we can't check if elements are hidden.

However, I see the same problem if I go out of reader mode, wait for the page to load, and go back in.

We should already be using the elements' height/width to determine whether we use them, so I don't know why that isn't detecting these elements. See https://dxr.mozilla.org/mozilla-central/source/toolkit/components/reader/ReaderMode.jsm#131-141 .

Comment 2

[Matthew N. [:MattN]]

(In reply to :Gijs Kruitbosch from comment #1)
> (In reply to Matthew N. [:MattN] from comment #0)
> > STR:
> > 1) Load the given URL
> 
> > #action-panel-rental-required and #action-panel-error have `display:none`
> > via the "hid" class so we could maybe use that to realize we shouldn't show
> > it.
> 
> We have no way of knowing that if we directly load the URI, because we XHR
> the article without rendering it, and so we can't check if elements are
> hidden.

We can easily check for a class of .hid though like we already do with .hidden. We should be careful to not look for "hid" as a proper substring of a class though.

Comment 3

[:Gijs (out for now; he/him)]

https://github.com/mozilla/readability/pull/260

Comment 4

[Matthew N. [:MattN]]

Attachment: https://github.com/mozilla/readability/pull/260

Comment 5

[Matthew N. [:MattN]]

Thanks. Do we wait to resolve the bug when an uplift from the Git repo occurs?

https://github.com/mozilla/readability/commit/a9597efc17003d88179f2a3289060c7cc1014f4c

Comment 6

[:Gijs (out for now; he/him)]

(In reply to Matthew N. [:MattN] (vacation Jan. 1 – 10) from comment #5)
> Thanks. Do we wait to resolve the bug when an uplift from the Git repo
> occurs?
> 
> https://github.com/mozilla/readability/commit/
> a9597efc17003d88179f2a3289060c7cc1014f4c

Yes. There's some other stuff I'd like to get in (they're outstanding PRs that need more substantial work than I've been able to find time for last year). If I don't get to it this week or the next I'll just uplift to fx-team as-is.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/f9691f3d45f8

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f9691f3d45f8