Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Show intermediate errors in pinned tabs #13610

Conversation

petemill
Copy link
Member

@petemill petemill commented Mar 26, 2018

Fix #8639
If loading a pinned tab errors, the error is not shown in the pinned tab, but is shown in a new tab which does not show the url which errors. The pinned tab then shows a non-branded error page. This can be a confusing UX as we've seen here, as it's not clear where the error tab has come from.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

On #8639 but also:

Steps to Reproduce

  1. Use borked profile from bad session state can result in permanently borked profile #13261 in which www.ebay.com cannot be loaded due to infinite redirects due to bad cookies
  2. Pin www.ebay.com
  3. Restart browser

Actual result:
2 additional tabs are created with errors that do not specify which url or tab caused those errors.
Pinned ebay.com tab shows unbranded error.

Expected result:
Error is shown in pinned ebay tab, and upon resolving of the error (clearing cookies), and restarting, ebay continues to load in a pinned tab.
- verify that when the error is resolved, the pinned tab has remembered which url it is supposed to load

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@codecov-io
Copy link

codecov-io commented Mar 26, 2018

Codecov Report

Merging #13610 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #13610      +/-   ##
==========================================
+ Coverage   56.74%   56.76%   +0.02%     
==========================================
  Files         285      285              
  Lines       28797    28782      -15     
  Branches     4755     4757       +2     
==========================================
- Hits        16341    16339       -2     
+ Misses      12456    12443      -13
Flag Coverage Δ
#unittest 56.76% <0%> (+0.02%) ⬆️
Impacted Files Coverage Δ
app/browser/tabs.js 23.93% <0%> (+0.14%) ⬆️

@bsclifton bsclifton added this to the Completed work milestone Mar 28, 2018
@alexwykoff alexwykoff modified the milestones: Completed work, 0.25.x Apr 10, 2018
@bsclifton bsclifton added the stale label Sep 8, 2018
@bsclifton bsclifton removed this from the 0.25.x (Nightly Channel) milestone Sep 8, 2018
@bsclifton bsclifton closed this Sep 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message in pinned tab will not be in the same tab
4 participants