Closed Bug 1151732 Opened 5 years ago Closed 1 year ago

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

Categories

(Toolkit :: Reader Mode, defect, P5)

38 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox38 --- wontfix
firefox39 --- wontfix
firefox40 --- wontfix
firefox67 --- fixed

People

(Reporter: avaida, Assigned: fpberkay, Mentored)

References

Details

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

Attachments

(1 file, 2 obsolete files)

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
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
Duplicate of this bug: 1166648
Whiteboard: [about-reader-ui]
Mentor: gijskruitbosch+bugs

I would like to work on this issue Gijs.

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

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

(In reply to lekhikadugtal from comment #3)

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

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.

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

(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

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

Attachment #9048680 - Attachment is obsolete: true
Attachment #9048681 - Attachment is obsolete: true

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

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
Status: NEW → RESOLVED
Closed: 1 year 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.