1395824 - Enable Table Borders in Reader View

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect, P3)

Description

[reinhart_previano]

Attachment: Screenshot_2017-09-01-12-35-56.png

User Agent: Mozilla/5.0 (Android 6.0; Mobile; rv:57.0) Gecko/57.0 Firefox/57.0
Build ID: 20170831100255

Steps to reproduce:

For some websites, such as Wikipedia, have long tables which are included in Reader View.
In this case, I visited a Wikipedia article about Firefox ( https://en.wikipedia.org/wiki/Firefox ) shown in Reader View ( about:reader?url=https%3A%2F%2Fen.m.wikipedia.org%2Fwiki%2FFirefox ).


Actual results:

The table borders are not visible, which cause confusion in long list of rows and columns. The screenshot/attachment shows the invisible border between supported Firefox version in Windows and macOS.


Expected results:

Since some articles may include long rows of tables, it is recommended to have the table borders present in Reader View. This can distinguish between cells and merged cells for reading convenience.

Comment 1

[Shreya]

Hi, I would like to work on this!
Could you please provide more information? Which files are affected? Thanks:)

Comment 2

[:Gijs (he/him)]

(In reply to Shreya from comment #1)

Hi, I would like to work on this!
Could you please provide more information? Which files are affected? Thanks:)

The main CSS file for reader mode lives at https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css . Instructions for building Firefox are at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build . You probably want to use an artifact build ( https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds ) as it's quicker and doesn't require a C++ compiler etc.

Comment 3

[Monika Maheshwari [:MonikaMaheshwari]]

Hi, I am outreachy applicant. I like to work on this issue.

Comment 4

[:Gijs (he/him)]

(In reply to Monika Maheshwari [:MonikaMaheshwari] from comment #3)

Hi, I am outreachy applicant. I like to work on this issue.

Well, let's give Shreya a chance to look at this first. Shreya, how are you getting on? Do you need help with anything?

Monika: perhaps you want to look at bug 1429442 instead?

Comment 5

[Shreya]

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

(In reply to Monika Maheshwari [:MonikaMaheshwari] from comment #3)

Hi, I am outreachy applicant. I like to work on this issue.

Well, let's give Shreya a chance to look at this first. Shreya, how are you getting on? Do you need help with anything?

Monika: perhaps you want to look at bug 1429442 instead?

Hey! Actually I'm getting errors in installing it locally, so trying to resolve it first

Comment 6

[:Gijs (he/him)]

(In reply to Shreya from comment #5)

Hey! Actually I'm getting errors in installing it locally, so trying to resolve it first

OK, let me know if I can help in some way.

Comment 7

[lekhikadugtal]

I would like to work on this issue.

Comment 8

[Monika Maheshwari [:MonikaMaheshwari]]

Hey if Shreya is not doing then I would like to work on it.

Comment 9

[:Gijs (he/him)]

Shreya, how are you getting on? Are you still working on this, and do you need help with anything?

Comment 10

[Shreya]

Hey sorry have been busy with hackathons and exams..I think I'll take up another issue since I have not been able to find time to work on this

Comment 11

[sonali18317]

Gijs, do we need to enable borders for desktop or Android or both? For Android I would have to change https://searchfox.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css right?

This is what the borders look like right now for Desktop mode. Please suggest any required changes.
https://pasteboard.co/Id5qGjP.png

Thanks :)

Comment 12

[sonali18317]

Attachment: Bug 1395824 Add css for table borders in aboutReader.css

Comment 13

[:Gijs (he/him)]

(In reply to sonali18317 from comment #11)

Gijs, do we need to enable borders for desktop or Android or both? For Android I would have to change https://searchfox.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css right?

Yes, we should update it for both, good point! (and yes, that's the right link)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d11e2cf33f3
Add css for table borders in aboutReader.css r=Gijs

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4d11e2cf33f3