Closed Bug 1287852 Opened 3 years ago Closed 3 years ago

Mark tabs.highlight as unsupported in the schema

Categories

(WebExtensions :: General, defect, P3)

defect

Tracking

(firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: wbamberg, Assigned: shatur, Mentored)

Details

(Keywords: dev-doc-complete, good-first-bug, Whiteboard: triaged)

Attachments

(3 files)

Attached file highlight.zip
I've attached a WebExtension that adds a button: click the button and it highlights the first tab in the current window.

This works in Chrome. On Firefox it does not work, and the following is logged to the console:

***
Error: An unexpected error occurred
Stack trace:
Async*highlightFirstTab/<@moz-extension://e7200d80-fa75-f541-8331-e275751841ab/background.js:47:5
Async*highlightFirstTab@moz-extension://e7200d80-fa75-f541-8331-e275751841ab/background.js:45:3

findPathInObject(...)[name] is not a function
***
Keywords: dev-doc-needed
Summary: tabs.highlight() does not work → tabs.highlight is defined in schema but not implemented
Mentor: kmaglione+bmo
Priority: -- → P3
Whiteboard: [good first bug] triaged
Keywords: good-first-bug
Whiteboard: [good first bug] triaged → triaged
I want to work in this bug
I want to work in this bug. Please assign this to me
Done. Please let me know if you have any questions.
Assignee: nobody → amitsin6h
(In reply to Kris Maglione [:kmag] from comment #3)
> Done. Please let me know if you have any questions.

How do I setup development environment to contribute in this project if any doc is available. Let me know
Amit, the basic build instructions are here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
Those instructions are slightly dated, when you get to the step where you create your mozconfig file, you should add the line:
ac_add_options --enable-artifact-builds
(you can read all about artifact builds here if you're interested: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds)
Beyond that, there are lots of general articles on MDN: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Articles_for_new_developers
But to get help with the specifics of this bug, you can ask questions here or even better, join the #webextensions channel on IRC.
(In reply to Andrew Swan [:aswan] from comment #5)
> Amit, the basic build instructions are here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Simple_Firefox_build
> Those instructions are slightly dated, when you get to the step where you
> create your mozconfig file, you should add the line:
> ac_add_options --enable-artifact-builds
> (you can read all about artifact builds here if you're interested:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Artifact_builds)
> Beyond that, there are lots of general articles on MDN:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Articles_for_new_developers
> But to get help with the specifics of this bug, you can ask questions here
> or even better, join the #webextensions channel on IRC.
 The file size is very large is their any other options
(In reply to Andrew Swan [:aswan] from comment #5)
> Amit, the basic build instructions are here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Simple_Firefox_build
> Those instructions are slightly dated, when you get to the step where you
> create your mozconfig file, you should add the line:
> ac_add_options --enable-artifact-builds
> (you can read all about artifact builds here if you're interested:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Artifact_builds)
> Beyond that, there are lots of general articles on MDN:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Articles_for_new_developers
> But to get help with the specifics of this bug, you can ask questions here
> or even better, join the #webextensions channel on IRC.
 The file size is very large is their any other options
(In reply to Amit singh from comment #7)
> (In reply to Andrew Swan [:aswan] from comment #5)
> > Amit, the basic build instructions are here:
> > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> > Build_Instructions/Simple_Firefox_build
> > Those instructions are slightly dated, when you get to the step where you
> > create your mozconfig file, you should add the line:
> > ac_add_options --enable-artifact-builds
> > (you can read all about artifact builds here if you're interested:
> > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> > Build_Instructions/Artifact_builds)
> > Beyond that, there are lots of general articles on MDN:
> > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> > Articles_for_new_developers
> > But to get help with the specifics of this bug, you can ask questions here
> > or even better, join the #webextensions channel on IRC.
>  The file size is very large is their any other options

It should be possible to build from a shallow git clone, or a tarball snapshot of the current source tree (either of which you can get from [1]), but that will require doing a full source build rather than an artifact build.

[1]: https://github.com/mozilla/gecko-dev
Attached patch 1287852.patchSplinter Review
Hi Kris!

I would like to this work on this bug if that's not a problem.

I have attached a patch on basis of my understanding[it may be wrong][Doesn't implement everything]. Also, how can I handle when multiple tabs are required to be highlighted.

Thanks!!
Flags: needinfo?(kmaglione+bmo)
Mentor: kmaglione+bmo → tomica
Flags: needinfo?(kmaglione+bmo) → needinfo?(tomica)
Hey Amit, can you let us know if still plan to work on this bug, since Tushar wants to give it a try.
Flags: needinfo?(amitsin6h)
(In reply to Tushar Saini (:shatur) from comment #9)
> I have attached a patch on basis of my understanding

Since Firefox doesn't actually support multi-tab selection, we don't actually plan to implement this feature.  So this bug will be about marking all mentions of highlighting as unsupported in the json schema.
Flags: needinfo?(tomica)
Summary: tabs.highlight is defined in schema but not implemented → Mark tabs.highlight as unsuported in the schema
(In reply to Tomislav Jovanovic :zombie from comment #10)
> Hey Amit, can you let us know if still plan to work on this bug, since
> Tushar wants to give it a try.

Two weeks without answer, switching the bug over to Tushar.
Assignee: amitsin6h → tushar.saini1285
Flags: needinfo?(amitsin6h)
Comment on attachment 8872284 [details]
Bug 1287852 - Mark tabs.highlight as unsupported in schema.

https://reviewboard.mozilla.org/r/143780/#review147668

You missed to change the android schema as well.

It would also be good to update the descriptions of other uses of `highlight` in the schema, to explain that we only support it as an alias for `active`.
Attachment #8872284 - Flags: review?(tomica)
Hey Tomislav!

Sorry for the delay updates. I was having my exams and was a little busy. I have made the changes, please have a look :)

Thanks!
Attachment #8872284 - Flags: review?(tomica)
Comment on attachment 8872284 [details]
Bug 1287852 - Mark tabs.highlight as unsupported in schema.

https://reviewboard.mozilla.org/r/143780/#review149896

Looks good.  Asking Kris for rs?
Attachment #8872284 - Flags: review+
Attachment #8872284 - Flags: review?(kmaglione+bmo)
Comment on attachment 8872284 [details]
Bug 1287852 - Mark tabs.highlight as unsupported in schema.

https://reviewboard.mozilla.org/r/143780/#review151526
Attachment #8872284 - Flags: review?(kmaglione+bmo) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/17c92027f70b
Mark tabs.highlight as unsupported in schema. r=kmag,zombie
Keywords: checkin-needed
Okay, I can reproduce this locally. Sorry, I should've pushed it to try first.

Updated the patch, Tomislav, can you look once again please.

Thanks
Flags: needinfo?(tushar.saini1285) → needinfo?(tomica)
Looks good, but you should push to try just in case.
Flags: needinfo?(tomica)
Summary: Mark tabs.highlight as unsuported in the schema → Mark tabs.highlight as unsupported in the schema
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4ea6ed24f779
Mark tabs.highlight as unsupported in schema. r=kmag,zombie
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4ea6ed24f779
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.