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

RESOLVED FIXED in Firefox 67

Status

()

defect
P5
normal
RESOLVED FIXED
4 years ago
18 days ago

People

(Reporter: avaida, Assigned: fpberkay, Mentored)

Tracking

38 Branch
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 affected, firefox39 affected, firefox40 affected, firefox67 fixed)

Details

(Whiteboard: [about-reader-ui] [bugday-20190327] )

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
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.
Priority: -- → P4

Comment 1

3 years ago
The reading list button is gone, renaming.
Priority: P4 → P5
Summary: The font panel from Reader View should disappear when clicking the Reading List sidebar button → The font panel from Reader View should disappear when clicking the Pocket sidebar button

Updated

3 years ago
Duplicate of this bug: 1166648

Updated

3 years ago
Whiteboard: [about-reader-ui]

Updated

2 months ago
Mentor: gijskruitbosch+bugs

Comment 3

2 months ago

I would like to work on this issue Gijs.

(Assignee)

Comment 4

2 months ago

(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

2 months ago

(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

2 months ago

(In reply to lekhikadugtal from comment #3)

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

(Assignee)

Comment 7

2 months ago
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

2 months ago

(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.

(Assignee)

Comment 9

2 months ago

(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

2 months ago

(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?

(Assignee)

Comment 12

2 months ago

Depends on D22196

Attachment #9048680 - Attachment is obsolete: true
Attachment #9048681 - Attachment is obsolete: true
(Assignee)

Comment 14

2 months ago

(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

a month ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b1f17a15a140
disappear panels in sidebar after clicking the Pocket sidebar button r=Gijs

Comment 16

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee: nobody → fpberkay
QA Whiteboard: [good first verify]

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

Whiteboard: [about-reader-ui] → [about-reader-ui] [bugday-20190327]
You need to log in before you can comment on or make changes to this bug.