Open Bug 1547408 Opened 4 years ago Updated 2 months ago

Parse Node Limit preference broken in Reader Mode

Categories

(Toolkit :: Reader Mode, defect, P5)

66 Branch
defect

Tracking

()

People

(Reporter: ascrod, Unassigned)

Details

Steps to Reproduce (Firefox Desktop)

  1. Navigate to about:config
  2. Set "reader.parse-node-limit" to "1"
  3. Navigate to https://en.wikipedia.org/wiki/Mozilla
  4. Click the Reader View button to enter Reader Mode

Expected Result
The page displays in Reader Mode with an error stating that the page cannot be loaded (because the page's node count is higher than the limit).

Actual Result
The page displays in Reader Mode without error.

Additional Details
The lazy getter for this pref in ReaderMode.jsm writes the value to the wrong property - it should be "parseNodeLimit", not "maxElemsToParse". It's a simple one-line fix.

Assignee: nobody → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

AFAICT this has always been broken (my patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1152219, https://hg.mozilla.org/mozilla-central/rev/1b47d308e51c, never assigned to this.parseNodeLimit either, unless the maxElemsToParse getter was called, which it never was). Re-enabling this will cause some pages to no longer be reader-mode-enabled on mobile. Jan, would you prefer we fix this that way, or should we just remove the pref given I've not heard of anyone complaining about slow reader mode on mobile?

Assignee: gijskruitbosch+bugs → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(jh+bugzilla)
Priority: -- → P5

As this only comes into play when actually entering reader mode anyway, and as you say nobody has complained about any related issues, I'd tend towards simply removing it, too.

Flags: needinfo?(jh+bugzilla)
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.