1417810 - Provide API to override the document colors

Assignee

Description

•

7 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171112125346

Steps to reproduce:

The newer WebExtensions API does not let us change the preference 'browser.display.document_color_use' to override document colors with those chosen by user.

Background:
Firefox's preference in Options - Fonts & Colors - Colors, where user can provide his/her own colors to override those in the document. The override switch has 3 choices - Always, Only with High Contrast themes and Never.
My OS is always in High Contrast mode and hence leaving the choice at 'Only with High Contrast themes' works for most part.
However some web pages are simply useless when the colors are overridden - the text is somehow the same color as the chosen background. Also most images do not show up at all.

My extension (using legacy AddonSDK) adds a toolbar button for easy switching among the 3 choices - Always, Only with High Contrast themes and Never. So on a web page that is not displayed well enough with HC theme, I can simply go to the toolbar button and choose the option Never - this will make Firefox use the document's colors instead of my choice and thereby I can view what's missing and provide any information in the input fields, if relevant.



Actual results:

API is unavailable thus my extension is of no use.

I have to open the Options page, browse to Fonts and Colors section (or search for 'Colors', open the Colors dialog and change the choice - this is too many clicks (not considering the scrolling or search input) and inconvenient.


Expected results:

Access to the "browser.display.document_color_use" preference through WebExtensions API would let users move between the choices easily with a simple toolbar button and only 2 clicks.

Satish Pasupuleti [:satishp]

Assignee

Updated

•

7 years ago

Component: Untriaged → WebExtensions: General

Product: Firefox → Toolkit

Satish Pasupuleti [:satishp]

Assignee

Updated

•

7 years ago

Blocks: 1363856

Bob Silverberg [:bsilverberg]

Comment 1

•

7 years ago

Adding to API triage.

Priority: -- → P5

Whiteboard: [design-decision-needed][browserSettings]

Mike Conca [:mconca]

Comment 2

•

7 years ago

Since this has to do with accessability, I'm bumping the priority to P2.

Severity: normal → enhancement

Priority: P5 → P2

Caitlin Neiman [:caitmuenster]

Comment 3

•

7 years ago

Hi psatish, this has been added to the agenda for the November 28, 2017 WebExtensions APIs triage meeting. Would you be able to join us? 

Here’s a quick overview of what to expect at the triage: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/Add-ons/Contribute/Triage
* Meeting agenda: https://docs.google.com/document/d/1MduEIKmXDdj3p94PJDrPPPYvbSfEdrrMF-UhMnHPqEQ/edit#heading=h.edrw957gm8hg
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Satish Pasupuleti [:satishp]

Assignee

Comment 4

•

7 years ago

Unfortunately no - my timezone is UTC+0530 making it 12AM and too late for me. I can provide more information when required though.

As an example, here's the link to a Amazon's product page that might help to understand the issue better - https://www.amazon.com/dp/B06XCM9LJ4

With the OS being in High Contrast mode and Firefox set to override colors only with High Contrast themes, as we scroll below, we can see dark patches where the images are present. Note that the choice of background color does not affect this at all - whichever color we choose to override the background, the images won't be visible. Toggling Colors to "Never" shows the images.

This sort of 'present-but-invisible' content is pretty common when browsing in High Contrast mode and I feel the need to frequently toggle between the Never and HC modes - hence the extension.

Michael Buckley

Comment 5

•

7 years ago

Just my 2 cents: if bug 1393022 was fixed a WebExtension could do its own implementation of changing the page colors, with the possibility of adding extra features beyond what this setting does.  Though adding the ability to change this would certainly make creating a simple extension very easy.

Tim Nguyen :ntim

Comment 6

•

7 years ago

This would fit well in the browserSettings API.

Caitlin Neiman [:caitmuenster]

Updated

•

7 years ago

Whiteboard: [design-decision-needed][browserSettings] → [design-decision-approved][browserSettings]

Tim Nguyen :ntim

Updated

•

7 years ago

Status: UNCONFIRMED → NEW

Ever confirmed: true

Tim Nguyen :ntim

Updated

•

6 years ago

Keywords: good-first-bug

Caitlin Neiman [:caitmuenster]

Comment 7

•

6 years ago

If this is your first contribution, please refer to https://wiki.mozilla.org/Add-ons/Contribute/Code on how to get started. 

Mentor: ntim

Mentor: ntim.bugs

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/228316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228316/

Tim Nguyen :ntim

Updated

•

6 years ago

Attachment #8959513 - Flags: review?(ntim.bugs)

Attachment #8959513 - Flags: review?(mixedpuppy)

Attachment #8959513 - Flags: review?(bob.silverberg)

Shane Caraveo (:mixedpuppy)

Comment 9

•

6 years ago

I'm at this point torn about this setting.  There is a more advanced version in the wings, see bug 1431473.

Flags: needinfo?(mconca)

Comment 10

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review234358

Thanks for the patch! This is a great start, I left a few comments that I think should be addressed :)

::: toolkit/components/extensions/ext-browserSettings.js:380
(Diff revision 1)
>            }),
> +        useDocumentColors: Object.assign(
> +          getSettingsAPI(
> +            extension, "useDocumentColors",
> +            () => {
> +              return Services.prefs.getIntPref("browser.display.document_color_use");

We shouldn't return 0, 1 or 2 here, these are internal values.

How about:
"always" for 2
"never" for 1
"high-contrast-only" for 0

::: toolkit/components/extensions/ext-browserSettings.js:384
(Diff revision 1)
> +            set: details => {
> +              if (![0, 1, 2].includes(details.value)) {
> +                throw new ExtensionError(
> +                  `${details.value} is not a valid value for useDocumentColors.`);
> +              }
> +              return ExtensionPreferencesManager.setSetting(
> +                extension.id, "useDocumentColors", details.value);

Same when setting the value, we should use more descriptive values than 0, 1 or 2.

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:181
(Diff revision 1)
>      "openSearchResultsInNewTabs", true,
>      {"browser.search.openintab": true});
>    await testSetting(
>      "openSearchResultsInNewTabs", false,
>      {"browser.search.openintab": false});
> -
> +  

nit: remove leading whitespace here and below

Tim Nguyen :ntim

Comment 11

•

6 years ago

(In reply to Shane Caraveo (:mixedpuppy) from comment #9)
> I'm at this point torn about this setting.  There is a more advanced version
> in the wings, see bug 1431473.

This bug is simply exposing a feature that has been available since very early versions of Firefox, and available in Firefox's preferences UI. As for bug 1431473, it seems to be exposing an internal WebRender flag that's not used anywhere else in Firefox.

So I would vote for only supporting this bug only, since it also covers the a11y use case in bug 1431473, and exposes a feature that's actively maintained and tested.

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/229646/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/229646/

Comment hidden (obsolete)

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review235376


Code analysis found 4 defects in this patch:
 - 4 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:181
(Diff revision 1)
>      "openSearchResultsInNewTabs", true,
>      {"browser.search.openintab": true});
>    await testSetting(
>      "openSearchResultsInNewTabs", false,
>      {"browser.search.openintab": false});
> -
> +  

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:191
(Diff revision 1)
> +    "useDocumentColors", 1,
> +    {"browser.display.document_color_use": 1});
> +  await testSetting(
> +    "useDocumentColors", 2,
> +    {"browser.display.document_color_use": 2});
> +  

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:349
(Diff revision 1)
>    async function background() {
>      await browser.test.assertRejects(
>        browser.browserSettings.contextMenuShowEvent.set({value: "bad"}),
>        /bad is not a valid value for contextMenuShowEvent/,
>        "contextMenuShowEvent.set rejects with an invalid value.");
> +    

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:354
(Diff revision 1)
> +    
> +    await browser.test.assertRejects(
> +      browser.browserSettings.useDocumentColors.set({value: 3}),
> +      /3 is not a valid value for useDocumentColors/,
> +      "useDocumentColors.set rejects with an invalid value.");
> +    

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

Comment hidden (obsolete)

Comment on attachment 8960889 [details]
Fixed issues reported in review of Bug 1417810
MozReview-Commit-ID: LfT56MLlQo9

https://reviewboard.mozilla.org/r/229646/#review235378


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/ext-browserSettings.js:381
(Diff revision 1)
>          useDocumentColors: Object.assign(
>            getSettingsAPI(
>              extension, "useDocumentColors",
>              () => {
> -              return Services.prefs.getIntPref("browser.display.document_color_use");
> +              let prefValue = Services.prefs.getIntPref("browser.display.document_color_use");
> +              return prefValue === 1 ? "never" : (prefValue === 2 ? "always" : "high-contrast-only");

Error: Do not nest ternary expressions. [eslint: no-nested-ternary]

::: toolkit/components/extensions/ext-browserSettings.js:391
(Diff revision 1)
> -              if (![0, 1, 2].includes(details.value)) {
> +              if (!["never", "always", "high-contrast-only"].includes(details.value)) {
>                  throw new ExtensionError(
>                    `${details.value} is not a valid value for useDocumentColors.`);
>                }
> +              let prefValue = 0; // initialize to 0 - auto/high-contrast-only
> +              if ("never" === details.value) {

Error: Expected literal to be on the right side of ===. [eslint: yoda]

::: toolkit/components/extensions/ext-browserSettings.js:393
(Diff revision 1)
>                    `${details.value} is not a valid value for useDocumentColors.`);
>                }
> +              let prefValue = 0; // initialize to 0 - auto/high-contrast-only
> +              if ("never" === details.value) {
> +                prefValue = 1;
> +              } else if ("always" === details.value) {

Error: Expected literal to be on the right side of ===. [eslint: yoda]

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/229650/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/229650/

Comment hidden (obsolete)

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review235390


Code analysis found 4 defects in this patch:
 - 4 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:181
(Diff revision 1)
>      "openSearchResultsInNewTabs", true,
>      {"browser.search.openintab": true});
>    await testSetting(
>      "openSearchResultsInNewTabs", false,
>      {"browser.search.openintab": false});
> -
> +  

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:191
(Diff revision 1)
> +    "useDocumentColors", 1,
> +    {"browser.display.document_color_use": 1});
> +  await testSetting(
> +    "useDocumentColors", 2,
> +    {"browser.display.document_color_use": 2});
> +  

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:349
(Diff revision 1)
>    async function background() {
>      await browser.test.assertRejects(
>        browser.browserSettings.contextMenuShowEvent.set({value: "bad"}),
>        /bad is not a valid value for contextMenuShowEvent/,
>        "contextMenuShowEvent.set rejects with an invalid value.");
> +    

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

::: toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js:354
(Diff revision 1)
> +    
> +    await browser.test.assertRejects(
> +      browser.browserSettings.useDocumentColors.set({value: 3}),
> +      /3 is not a valid value for useDocumentColors/,
> +      "useDocumentColors.set rejects with an invalid value.");
> +    

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

Comment hidden (obsolete)

Comment on attachment 8960889 [details]
Fixed issues reported in review of Bug 1417810
MozReview-Commit-ID: LfT56MLlQo9

https://reviewboard.mozilla.org/r/229646/#review235392


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/ext-browserSettings.js:381
(Diff revision 1)
>          useDocumentColors: Object.assign(
>            getSettingsAPI(
>              extension, "useDocumentColors",
>              () => {
> -              return Services.prefs.getIntPref("browser.display.document_color_use");
> +              let prefValue = Services.prefs.getIntPref("browser.display.document_color_use");
> +              return prefValue === 1 ? "never" : (prefValue === 2 ? "always" : "high-contrast-only");

Error: Do not nest ternary expressions. [eslint: no-nested-ternary]

::: toolkit/components/extensions/ext-browserSettings.js:391
(Diff revision 1)
> -              if (![0, 1, 2].includes(details.value)) {
> +              if (!["never", "always", "high-contrast-only"].includes(details.value)) {
>                  throw new ExtensionError(
>                    `${details.value} is not a valid value for useDocumentColors.`);
>                }
> +              let prefValue = 0; // initialize to 0 - auto/high-contrast-only
> +              if ("never" === details.value) {

Error: Expected literal to be on the right side of ===. [eslint: yoda]

::: toolkit/components/extensions/ext-browserSettings.js:393
(Diff revision 1)
>                    `${details.value} is not a valid value for useDocumentColors.`);
>                }
> +              let prefValue = 0; // initialize to 0 - auto/high-contrast-only
> +              if ("never" === details.value) {
> +                prefValue = 1;
> +              } else if ("always" === details.value) {

Error: Expected literal to be on the right side of ===. [eslint: yoda]

Tim Nguyen :ntim

Comment 18

•

6 years ago

Can you combine the 3 patches together ? It's easier to review this way.

Satish Pasupuleti [:satishp]

Assignee

Comment 19

•

6 years ago

I apologize for the confusing commits.
My idea was to submit to the same review - as far as I understood MozReview docs, using the same bug reference should give the same ReviewID but I found that the review IDs were new for every commit.
I am not sure of how to patch them together; is there a page with instructions? Thanks.

Shane Caraveo (:mixedpuppy)

Comment 20

•

6 years ago

Go to the patch in reviewboard you want to update, you'll see something like "MozReview-Commit-ID: LfT56MLlQo9".  Add that to the commit message:

"""
Bug xxx - description

longer description

MozReview-Commit-ID: LfT56MLlQo9
"""

Comment hidden (obsolete)

Comment on attachment 8960901 [details]
Fixed eslint reports - Bug 1417810
MozReview-Commit-ID: LfT56MLlQo9

https://reviewboard.mozilla.org/r/229650/#review235724

::: commit-message-e0db9:3
(Diff revision 1)
> +Fixed eslint reports - Bug 1417810 r?ntim
> +
> +MozReview-Commit-ID: CYp1hL793sE

MozReview-Commit-ID: LfT56MLlQo9

Comment hidden (obsolete)

Comment on attachment 8960901 [details]
Fixed eslint reports - Bug 1417810
MozReview-Commit-ID: LfT56MLlQo9

https://reviewboard.mozilla.org/r/229650/#review236042

Satish Pasupuleti [:satishp]

Assignee

Updated

•

6 years ago

Attachment #8960901 - Flags: review?(psatish) → review-

Satish Pasupuleti [:satishp]

Assignee

Comment 23

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #18)
> Can you combine the 3 patches together ? It's easier to review this way.

I added the Commit-ID but I am not sure I did it right. Can you confirm if you see the review request and the changes consolidated?

Tim Nguyen :ntim

Comment 24

•

6 years ago

You need to combine all 3 commits in one, right now, there are still 3 separate commits.

Satish Pasupuleti [:satishp]

Assignee

Updated

•

6 years ago

Attachment #8960889 - Attachment description: Fixed issues reported in review of Bug 1417810 → Fixed issues reported in review of Bug 1417810 MozReview-Commit-ID: LfT56MLlQo9

Satish Pasupuleti [:satishp]

Assignee

Updated

•

6 years ago

Attachment #8960901 - Attachment description: Fixed eslint reports - Bug 1417810 → Fixed eslint reports - Bug 1417810 MozReview-Commit-ID: LfT56MLlQo9

Comment hidden (mozreview-request)

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228316/diff/1-2/

Satish Pasupuleti [:satishp]

Assignee

Updated

•

6 years ago

Attachment #8960889 - Attachment is obsolete: true

Attachment #8960889 - Flags: review?(ntim.bugs)

Satish Pasupuleti [:satishp]

Assignee

Updated

•

6 years ago

Attachment #8960901 - Attachment is obsolete: true

Attachment #8960901 - Flags: review?(ntim.bugs)

Tim Nguyen :ntim

Comment 26

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review236698

::: toolkit/components/extensions/ext-browserSettings.js:383
(Diff revision 2)
> +              } else if (prefValue === 2) {
> +                return "always";
> +              }
> +              return "high-contrast-only";
> +            }

Looking more at this, "overrideDocumentColors" would be a more appropriate name.

Since in the "never" case, the document colors are chosen, while in the "always" case, the document colors are overriden.

Attachment #8959513 - Flags: review?(ntim.bugs)

Tim Nguyen :ntim

Updated

•

6 years ago

Attachment #8959513 - Flags: review?(mixedpuppy)

Tim Nguyen :ntim

Comment 27

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #26)
> Comment on attachment 8959513 [details]
> Bug 1417810 - Added API to override document colors
> 
> https://reviewboard.mozilla.org/r/228316/#review236698
> 
> ::: toolkit/components/extensions/ext-browserSettings.js:383
> (Diff revision 2)
> > +              } else if (prefValue === 2) {
> > +                return "always";
> > +              }
> > +              return "high-contrast-only";
> > +            }
> 
> Looking more at this, "overrideDocumentColors" would be a more appropriate
> name.

To be clear, this comment applies to "useDocumentColors", not to "high-contrast-only"

Comment hidden (mozreview-request)

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228316/diff/2-3/

Shane Caraveo (:mixedpuppy)

Comment 29

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review237638

This is looking fine.  I've kicked off a try from rb, I want to see the android tests pass.  As well, this needs a followup bug to handle the extension setting UI in about:prefs, and we should probably hold off landing this until we know the UI work can land in the same version of firefox.

Shane Caraveo (:mixedpuppy)

Updated

•

6 years ago

Blocks: 1449665

Shane Caraveo (:mixedpuppy)

Updated

•

6 years ago

Attachment #8959513 - Flags: review?(ntim.bugs)

Shane Caraveo (:mixedpuppy)

Comment 30

•

6 years ago

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

getting this on my review queue.

Attachment #8959513 - Flags: review?(ntim.bugs)

Attachment #8959513 - Flags: review?(mixedpuppy)

Mike Conca [:mconca]

Comment 31

•

6 years ago

> (In reply to Tim Nguyen :ntim from comment #11)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #9)
> > I'm at this point torn about this setting.  There is a more advanced version
> > in the wings, see bug 1431473.
> 
> This bug is simply exposing a feature that has been available since very
> early versions of Firefox, and available in Firefox's preferences UI. As for
> bug 1431473, it seems to be exposing an internal WebRender flag that's not
> used anywhere else in Firefox.
> 
> So I would vote for only supporting this bug only, since it also covers the
> a11y use case in bug 1431473, and exposes a feature that's actively
> maintained and tested.

This browserSetting API exposes a feature of the browser available to users via preferences. In the past, this type of WebExtension API has typically been approved, and that should be the case here as well. I recommend we land this patch.

As for the relation to bug 1431373, it has slightly different use cases, most related to accessibility, but not entirely. The merits of landing it will be discussed in that bug, separate from this one.

Flags: needinfo?(mconca)

Mark Striemer [:mstriemer]

Comment 32

•

6 years ago

The UI updates for preferences are tracked in bug 1449665. If you'd like to work on that Satish I've added some pointers on how to get started in that bug and can help answer any questions you may have.

Thank you for your contribution!

Shane Caraveo (:mixedpuppy)

Comment 33

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review237722

Attachment #8959513 - Flags: review?(mixedpuppy) → review+

Tim Nguyen :ntim

Comment 34

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review237726

Looks good to me, thanks for contributing! mixedpuppy should give a final pass on this as he is a peer :)

Try looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e053ea0e97fa2a728c5e99648db2e6b6c605782

Attachment #8959513 - Flags: review?(ntim.bugs)

Shane Caraveo (:mixedpuppy)

Comment 35

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #34)

> mixedpuppy should give a final pass on this as he is a peer :)

I just did, I'll make rb land it.  Thanks Satish!

Tim Nguyen :ntim

Comment 36

•

6 years ago

mozreview-review

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

https://reviewboard.mozilla.org/r/228316/#review237728

Attachment #8959513 - Flags: review+

Tim Nguyen :ntim

Updated

•

6 years ago

Assignee: nobody → psatish

Mozilla Autoland

Comment 37

•

6 years ago

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 9211a415dff9cfbf8434773a055cd50cccdcfbed -d 02213fd3a9c8: rebasing 454936:9211a415dff9 "Bug 1417810 - Added API to override document colors r=mixedpuppy" (tip)
merging toolkit/components/extensions/ext-browserSettings.js
merging toolkit/components/extensions/schemas/browser_settings.json
merging toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
warning: conflicts while merging toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Tim Nguyen :ntim

Comment 38

•

6 years ago

Satish, could you please update your patch on top of the latest source code ?

Let us know if you need help doing so.

Thanks!

Flags: needinfo?(psatish)

Satish Pasupuleti [:satishp]

Assignee

Comment 39

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #38)
> Satish, could you please update your patch on top of the latest source code ?
> 
> Let us know if you need help doing so.
> 
> Thanks!

Yes, I think I will need help. I ran the 'hg pull' and 'hg rebase' commands and merged my changes with the latest in kDiff3. However after saving that, 'hg commit' and 'hg rebase --continue' show there's nothing to commit or no rebase in progress.

The file with unresolved conflicts is merged and resolved - how can I push this upstream?

Thanks

Tim Nguyen :ntim

Comment 40

•

6 years ago

Does `hg log` show your commit ? If so, you can just push it to mozreview.

Satish Pasupuleti [:satishp]

Assignee

Comment 41

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #40)
> Does `hg log` show your commit ? If so, you can just push it to mozreview.

No. It still shows the commit from 2 days ago.

Tim Nguyen :ntim

Comment 42

•

6 years ago

Could you share your output of `hg log` ?

Satish Pasupuleti [:satishp]

Assignee

Comment 43

•

6 years ago

(In reply to Tim Nguyen :ntim from comment #42)
> Could you share your output of `hg log` ?

satish@win10pc /d/mozilla/source/mozilla-central
$ hg log toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js

changeset:   410511:dec291fcab76
tag:         tip
user:        Satish <psatish@live.in>
date:        Tue Mar 27 12:01:44 2018 +0530
summary:     Bug 1417810 - Added API to override document colors r?ntim

changeset:   409988:aee255d8336c
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Fri Jan 19 12:59:53 2018 -0500
summary:     Bug 1344749 - Expose API to customize where new tabs open, r=dao

changeset:   409904:a27611cba7d7
user:        Hector Zhao <bzhao@mozilla.com>
date:        Tue Mar 06 17:11:49 2018 +0800
summary:     Bug 1435142 - Part 3: expose closeTabByDblclick to WebExtension. r=mixedpuppy

changeset:   409696:8b7876354e56
user:        Narcis Beleuzu <nbeleuzu@mozilla.com>
date:        Fri Mar 23 19:45:14 2018 +0200
summary:     Backed out 3 changesets (bug 1435142) for browser-chrome failures on browser_preloadedBrowser_zoom.js

changeset:   409668:7334da12de35
user:        Hector Zhao <bzhao@mozilla.com>
date:        Tue Mar 06 17:11:49 2018 +0800
summary:     Bug 1435142 - Part 3: expose closeTabByDblclick to WebExtension. r=mixedpuppy

changeset:   409012:c7d3eda7dedc
user:        Noemi Erli <nerli@mozilla.com>
date:        Tue Mar 20 15:38:04 2018 +0200
summary:     Backed out 3 changesets (bug 1435142) for failing in browser/base/content/test/tabs/browser_close_tab_by_dblclick.js on a CLOSED TREE

changeset:   409005:9193e64cfe29
user:        Hector Zhao <bzhao@mozilla.com>
date:        Tue Mar 06 17:11:49 2018 +0800
summary:     Bug 1435142 - Part 3: expose closeTabByDblclick to WebExtension. r=mixedpuppy

changeset:   408806:ea5c6ffc930e
user:        Dorel Luca <dluca@mozilla.com>
date:        Mon Mar 19 17:54:01 2018 +0200
summary:     Backed out 3 changesets (bug 1435142) for Mochitest failure on browser/base/content/test/tabs/browser_close_tab_by_dblclick.js

changeset:   408801:bcde232543f8
user:        Hector Zhao <bzhao@mozilla.com>
date:        Tue Mar 06 17:11:49 2018 +0800
summary:     Bug 1435142 - Part 3: expose closeTabByDblclick to WebExtension. r=mixedpuppy

changeset:   406462:e7bfe2b00511
user:        Dão Gottwald <dao@mozilla.com>
date:        Sat Mar 03 15:04:51 2018 +0100
summary:     Backed out changeset 0d138609abfd (bug 1344749)

changeset:   406139:0d138609abfd
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Fri Jan 19 12:59:53 2018 -0500
summary:     Bug 1344749 - Expose API to customize where new tabs open, r=dao,mixedpuppy

changeset:   401543:e6a7b5e11ba8
user:        Kris Maglione <maglione.k@gmail.com>
date:        Mon Jan 29 15:20:18 2018 -0800
summary:     Bug 1431533: Part 5a - Auto-rewrite code to use ChromeUtils import methods. r=florian

changeset:   401421:c6e0fe339cb1
user:        Cosmin Sabou <csabou@mozilla.com>
date:        Tue Jan 30 07:17:48 2018 +0200
summary:     Backed out 3 changesets (bug 1431533) for Android mochitest failures on testEventDispatcher on a CLOSED TREE

changeset:   401376:34c999fa006b
user:        Kris Maglione <maglione.k@gmail.com>
date:        Mon Jan 29 15:20:18 2018 -0800
summary:     Bug 1431533: Part 5a - Auto-rewrite code to use ChromeUtils import methods. r=florian

changeset:   401362:505fc52bd608
user:        Brindusan Cristian <cbrindusan@mozilla.com>
date:        Tue Jan 30 02:32:43 2018 +0200
summary:     Backed out 2 changesets (bug 1431533) for ESlint failures on a CLOSED TREE

changeset:   401358:12fc4dee861c
user:        Kris Maglione <maglione.k@gmail.com>
date:        Mon Jan 29 15:20:18 2018 -0800
summary:     Bug 1431533: Part 5a - Auto-rewrite code to use ChromeUtils import methods. r=florian

changeset:   400770:cc87ad81ff86
user:        Kris Maglione <maglione.k@gmail.com>
date:        Wed Jan 24 22:04:59 2018 -0800
summary:     Backed out 3 changesets (bug 1431533) for Android mochitest bustage. CLOSED TREE

changeset:   400767:d4a7c018420e
user:        Kris Maglione <maglione.k@gmail.com>
date:        Wed Jan 24 15:48:47 2018 -0800
summary:     Bug 1431533: Part 5a - Auto-rewrite code to use ChromeUtils import methods. r=florian

changeset:   399804:891afc6d66f1
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Thu Jan 11 15:22:03 2018 -0500
summary:     Bug 1425535 - Implement browserSettings.proxyConfig API, r=mixedpuppy

changeset:   397122:f418166c70b8
user:        Florian Quèze <florian@queze.net>
date:        Thu Dec 21 11:10:23 2017 +0100
summary:     Bug 1421992 - script-generated patch to replace do_execute_soon, do_print and do_register_cleanup with executeSoon, info and registerCleanupFunction, rs=Gijs.

changeset:   395878:0f8c30988da2
user:        Kris Maglione <maglione.k@gmail.com>
date:        Tue Nov 28 14:13:59 2017 -0800
summary:     Bug 1421459: Update to ESLint 4 "indent" rule. r=aswan

changeset:   394264:77229a598e9f
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Wed Nov 29 09:49:46 2017 -0500
summary:     Bug 1420969 - API to open search results in new tab, r=mixedpuppy

changeset:   394263:5a46e18c726d
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Tue Nov 28 09:20:31 2017 -0500
summary:     Bug 1420974 - API to open bookmarks in new tab, r=mixedpuppy

changeset:   393486:d4779e0fdbbb
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Tue Nov 21 12:33:15 2017 -0500
summary:     Bug 1419426 - Implement browserSettings.contextMenuShowEvent, r=kmag

changeset:   393369:2da6fb2d843a
user:        Brindusan Cristian <cbrindusan@mozilla.com>
date:        Thu Nov 23 16:19:37 2017 +0200
summary:     Backed out changeset e8d475fed206 (bug 1419426) for opt-test-verify failures on extensions/test/xpcshell/test_ext_browserSettings.js. r=backout a=backout on a CLOSED TREE

changeset:   393356:e8d475fed206
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Tue Nov 21 12:33:15 2017 -0500
summary:     Bug 1419426 - Implement browserSettings.contextMenuShowEvent, r=kmag

changeset:   388699:2ac09a374b69
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Wed Oct 18 17:46:38 2017 -0400
summary:     Bug 1364942 - Allow WebExtensions to disable Web API notifications, r=mixedpuppy

changeset:   379741:39a2a87fc100
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Wed Sep 06 16:46:38 2017 -0400
summary:     Bug 1364972 - Allow WebExtensions to disable animated images, r=mixedpuppy

changeset:   379515:9047e2077ec7
user:        Wes Kocher <wkocher@mozilla.com>
date:        Thu Sep 07 12:31:11 2017 -0700
summary:     Backed out changeset 674775dda8ad (bug 1364972) for eslint failures a=backout

changeset:   379514:674775dda8ad
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Wed Sep 06 16:46:38 2017 -0400
summary:     Bug 1364972 - Allow WebExtensions to disable animated images, r=mixedpuppy

changeset:   372334:b89bf45144d8
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Mon Jul 31 16:46:36 2017 -0400
summary:     Bug 1339550 - Implement browser.settings.allowPopupsForUserEvents, r=aswan

changeset:   364774:4383e27b5216
user:        Bob Silverberg <bsilverberg@mozilla.com>
date:        Wed May 24 13:17:42 2017 -0400
summary:     Bug 1364936 - Allow WebExtensions to disable the browser cache, r=aswan

Tim Nguyen :ntim

Comment 44

•

6 years ago

That looks fine to me, you should just be able to push what you have to mozreview.

Comment hidden (mozreview-request)

Comment on attachment 8959513 [details]
Bug 1417810 - Added API to override document colors

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228316/diff/3-4/

Satish Pasupuleti [:satishp]

Assignee

Comment 46

•

6 years ago

Done. Thanks, looks like it worked!!

Flags: needinfo?(psatish)

Pulsebot

Comment 47

•

6 years ago

Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/430a1c231025
Added API to override document colors r=mixedpuppy,ntim

Tim Nguyen :ntim

Comment 48

•

6 years ago

Congrats on your first contribution! It should be in Nightly some time tomorrow :)

Andrei Ciure[:aciure]

Comment 49

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/430a1c231025

Status: NEW → RESOLVED

Closed: 6 years ago

status-firefox61: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla61

Caitlin Neiman [:caitmuenster]

Comment 50

•

6 years ago

Yay! Congrats on landing this, Satish! This has been added to our add-ons recognition wiki: https://wiki.mozilla.org/Add-ons/Contribute/Recognition#March_2018

Would you be interested in creating a profile on mozillians.org? I'd love to vouch for you!

Satish Pasupuleti [:satishp]

Assignee

Comment 51

•

6 years ago

Thank you everyone for the kind words. Tbh, though, this was quite simple. All I did was copy-paste few lines :-)

Tim Nguyen :ntim

Updated

•

6 years ago

Keywords: dev-doc-needed

marius.santa

Comment 52

•

6 years ago

Could you please provide a link or xpi for your extension in order to test it?

Flags: needinfo?(psatish)

Satish Pasupuleti [:satishp]

Assignee

Comment 53

•

6 years ago

Attached file override_document_colors-0.0.4-an+fx.xpi — Details

Attached a very simple (and crude) extension that uses this new setting to toggle the document colors.

Flags: needinfo?(psatish)

Victor Carciu

Comment 54

•

6 years ago

Verified as fixed in Latest Nightly using Windows 10x64 and I will attach a postfix screenshot. Also, I can confirm that the selector is not working in FF 59.02.

Status: RESOLVED → VERIFIED

Victor Carciu

Comment 55

•

6 years ago

Attached image Postfix video — Details

Victor Carciu

Updated

•

6 years ago

status-firefox61: fixed → verified

Will Bamberg [:wbamberg]

Comment 56

•

6 years ago

Docs -> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings/overrideDocumentColors

Please let me know if this covers it.

Flags: needinfo?(bob.silverberg)

Bob Silverberg [:bsilverberg]

Comment 57

•

6 years ago

LGTM, thanks Will.

Flags: needinfo?(bob.silverberg)

Will Bamberg [:wbamberg]

Updated

•

6 years ago

Keywords: dev-doc-needed → dev-doc-complete

BMO Automation

Updated

•

6 years ago

Product: Toolkit → WebExtensions

Bug 1417810 - Added API to override document colors 6 years ago Satish Pasupuleti [:satishp] 59 bytes, text/x-review-board-request	ntim : review+	Details
Fixed issues reported in review of Bug 1417810 MozReview-Commit-ID: LfT56MLlQo9 6 years ago Satish Pasupuleti [:satishp] 59 bytes, text/x-review-board-request		Details
Fixed eslint reports - Bug 1417810 MozReview-Commit-ID: LfT56MLlQo9 6 years ago Satish Pasupuleti [:satishp] 59 bytes, text/x-review-board-request	satishp : review-	Details
override_document_colors-0.0.4-an+fx.xpi 6 years ago Satish Pasupuleti [:satishp] 11.75 KB, application/x-xpinstall		Details
Postfix video 6 years ago Victor Carciu 412.90 KB, image/gif		Details