1151732 - The font panel from Reader View should disappear when clicking the Pocket sidebar button

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect, P5)

Description

[Andrei Vaida [:avaida]]

Note: this is a follow-up issue filed for Bug 1137211.

Reproducible on:
Nightly 40.0a1 (2015-04-06), Aurora 39.0a2 (2015-04-06), Beta 38.0b2-build1 (20150406174117)

Affected platforms:
Windows 7 (x64), Ubuntu 14.04 (x64), Mac OS X 10.9.5

Steps to reproduce:
1. Launch Firefox.
2. Open a Reader View compatible page - (e.g.) http://www.bbc.com/future/story/20150402-do-colours-really-change-our-mood
3. Click the "Enter Reader View" button from the Location Bar.
4. Click the "Type controls" (Aa) button from Reader View's controls bar.
5. Click the "Open Reading List" button below it.

Expected result:
Once the "Open Reading List" button is clicked, the font panel is dismissed.

Actual result:
The font panel is not dismissed.

Comment 1

[:Gijs (he/him)]

The reading list button is gone, renaming.

Comment 3

[lekhikadugtal]

I would like to work on this issue Gijs.

Comment 4

[Berkay Barlas]

(In reply to :Gijs (he/him) from comment #1)

The reading list button is gone, renaming.

Thank you for your suggestion Gijs. I reproduced and understand the problem but I couldn't find the exact folder related to reader mode. Could you help me ?

Comment 5

[:Gijs (he/him)]

(In reply to lekhikadugtal from comment #3)

I would like to work on this issue Gijs.

Unfortunately I suggested this bug to Berkay Barlas in bug 1187696 so they should probably have a crack at it first.

(In reply to Berkay Barlas from comment #4)

(In reply to :Gijs (he/him) from comment #1)

The reading list button is gone, renaming.

Thank you for your suggestion Gijs. I reproduced and understand the problem but I couldn't find the exact folder related to reader mode. Could you help me ?

https://searchfox.org/mozilla-central/source/toolkit/components/reader/AboutReader.jsm

Specifically, https://searchfox.org/mozilla-central/rev/92d11a33250a8e368c8ca3e962e15ca67117f765/toolkit/components/reader/AboutReader.jsm#249 is supposed to handle closing the dropdowns. The question will be why the click on the pocket button doesn't trigger this helper method.

Comment 6

[:Gijs (he/him)]

(In reply to lekhikadugtal from comment #3)

Perhaps have a look at bug 1148052 instead, if you're comfortable with CSS.

Comment 7

[Berkay Barlas]

this._setupButton(message.data.id, button => {
            this._mm.sendAsyncMessage("Reader:Clicked-" + button.dataset.buttonid, { article: this._article });
            this._closeDropdowns();
          });

adding "this._closeDropdowns();" to line 221 solves to problem.

But I also realized that clicking on Pocket sidebar button does not call event handler('click') of AboutReader.

Comment 8

[:Gijs (he/him)]

(In reply to Berkay Barlas from comment #7)

But I also realized that clicking on Pocket sidebar button does not call event handler('click') of AboutReader.

Right. It would probably be better to fix that, instead.

So the reason the handler isn't invoked must be that it's somehow not getting the click event. Have a look at the code in _setupButton and see if you can figure out why that might be the case, and if we can change how we register the "click" handler for AboutReader to work better for this case.

Comment 9

[Berkay Barlas]

(In reply to :Gijs (he/him) from comment #8)

Right. It would probably be better to fix that, instead.

Okay,if we want to get continue event updates from button, I think we should delete the stopPropagation method at the line 954.

https://searchfox.org/mozilla-central/rev/92d11a33250a8e368c8ca3e962e15ca67117f765/toolkit/components/reader/AboutReader.jsm#954

Comment 10

[:Gijs (he/him)]

(In reply to Berkay Barlas from comment #9)

(In reply to :Gijs (he/him) from comment #8)

Right. It would probably be better to fix that, instead.

Okay,if we want to get continue event updates from button, I think we should delete the stopPropagation method at the line 954.

https://searchfox.org/mozilla-central/rev/92d11a33250a8e368c8ca3e962e15ca67117f765/toolkit/components/reader/AboutReader.jsm#954

Yep, I think that'd work. Can you submit a patch to phabricator that does that?

Comment 11

[Berkay Barlas]

Attachment: Bug 1151732 disappear panels in sidebar after clicking the Pocket sidebar button

Comment 12

[Berkay Barlas]

Attachment: Bug 1151732 disappear panels in sidebar after clicking the Pocket sidebar button

Depends on D22196

Comment 13

[Berkay Barlas]

Attachment: Bug 1151732 disappear panels in sidebar after clicking the Pocket sidebar button

Comment 14

[Berkay Barlas]

(In reply to :Gijs (he/him) from comment #10)

Can you submit a patch to phabricator that does that?

I think I finally able to submit a patch to phabricator :D. Thank you for mentorship Gijs!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1f17a15a140
disappear panels in sidebar after clicking the Pocket sidebar button r=Gijs

Comment 16

[Andreea Pavel [:apavel]]

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

Comment 17

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 42.0a1 (2015-06-30) on Windows 7, 64 Bit.
The fix of this bug is verified with latest Beta 67.0b6!

Build ID : 20190328152334
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0