769642 - Reader Mode: Hide toolbar while loading content

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Lucas Rocha (:lucasr)]

Toolbar actions should not be available while reader content is being loaded.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Only show toolbar once the reader content is shown

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 637884 [details] [diff] [review]
Only show toolbar once the reader content is shown

Patch looks fine, but I had a question about some previous code

>diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js

>   _setToolbarVisibility: function Reader_setToolbarVisibility(visible) {

>     if (visible)
>       toolbarElement.display = "block";
>
>     toolbarElement.classList.toggle("toolbar-hidden");

Why do we need this check: toolbarElement.display = "block";

This should be enough: toolbarElement.classList.toggle(...);

and even so, we are missing the ".style" property, right?

r+ but let's address the old code

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 637884 [details] [diff] [review]
> Only show toolbar once the reader content is shown
> 
> Patch looks fine, but I had a question about some previous code
> 
> >diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js
> 
> >   _setToolbarVisibility: function Reader_setToolbarVisibility(visible) {
> 
> >     if (visible)
> >       toolbarElement.display = "block";
> >
> >     toolbarElement.classList.toggle("toolbar-hidden");
> 
> Why do we need this check: toolbarElement.display = "block";
> 
> This should be enough: toolbarElement.classList.toggle(...);
> 
> and even so, we are missing the ".style" property, right?
> 
> r+ but let's address the old code

This code is removed in my patch for bug 766961.

Comment 4

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/integration/mozilla-inbound/rev/55d0b0f169fc

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/55d0b0f169fc