Closed Bug 769572 Opened 12 years ago Closed 8 years ago

GCLI needs a CSSDOC command

Categories

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 768469
Future

People

(Reporter: jwalker, Unassigned, Mentored)

References

Details

(Whiteboard: [gclicommands][good first bug])

Attachments

(2 files, 2 obsolete files)

Latest source that I know of:
https://gist.github.com/1d943933ffb43a72946d

Outstanding:
- Decide if we should make a more general mdn command which can search for
  things other than CSS properties.
  Paul kicked off an 'I'm feeling lucky' thing IIRC.
- Localization
- Testing
Related bugs:
- Bug 768456 - Implement a "I'm feeling lucky" mechanism
- Bug 768469 - [gcli] implement a "mdn" command
Filter on teabags
Whiteboard: [gclicommands]
mgoodwin said that we need to add iframe.setAttribute("style", "content"); to the iframe.
I probably meant iframe.setAttribute("type", "content")

...although I now understand why you made a comment about CSS in IRC :)
Triage, filter on TEABAGS
Target Milestone: Firefox 16 → Firefox 17
Triage: Filter on the TRIAGE keyword.
Target Milestone: Firefox 17 → Future
Triage
Whiteboard: [gclicommands] → [gclicommands][good first bug][mentor=jwalker]
New component triage. Filter on "Lobster Thermidor aux crevettes with a Mornay sauce"
Component: Developer Tools: Console → Developer Tools: Graphic Commandline and Toolbar
Assignee: mratcliffe → nobody
Hi, 

I am willing to work on this bug. So please I request you to assign it to me.

Thanks in Advance,

Regards,
A. Anup
(In reply to Anup Allamsetty from comment #9)
> Hi, 
> 
> I am willing to work on this bug. So please I request you to assign it to me.
> 
> Thanks in Advance,
> 
> Regards,
> A. Anup

Done
Assignee: nobody → allamsetty.anup
Assignee: allamsetty.anup → nobody
can i work on this?
(In reply to Hubert B Manilla from comment #11)
> can i work on this?

Certainly - I've assigned it to you. Thanks!
Do you need any help?
Assignee: nobody → b4bomsy
hi joe,
Thanks.
I'm assuming i can get some information from your blog
http://incompleteness.me/blog/ and from https://bugzilla.mozilla.org/show_bug.cgi?id=724055
to work on this?
Hi Hubert - If we were to get bug 768456 fixed, then that would greatly help this, bug, so perhaps it might be a good idea to vote on that bug as a first step.
Since this is not actively worked on, removing the assignee.
Assignee: b4bomsy → nobody
I've started implementing this. This is my first devtools bug so I've probably made some mistakes.

I based the implementation off the gist in comment 1. The changes I made:

* Use the raw version of MDN instead of loading the whole page. This is seriously faster (on the order of 10x some days). You do this by appending '?raw&macros' to the URL: https://developer.mozilla.org/en-US/docs/Web/CSS/display?raw&macros
* I also use the 'DOMContentLoaded' event instead of 'load', though since we're getting the raw page (without any JS) I doubt it makes much difference.
* I tweaked the output so it shows a header, a link right below it, and then the description with some padding at the top.

Problems so far:

* Links aren't clickable for some reason
* A really odd behavior is when you search for a keyword that 404s on MDN. The whole browser window is replaced with the MDN 404 page. I can't figure out why.
* The style of the popup needs some input.

I'm going to just attach my WIP patch. Is that right?
Attached patch 769572-cssdoc-glci.patch (obsolete) — Splinter Review
Attachment #8381686 - Flags: review?(jwalker)
Assignee: nobody → jlong
In case it's useful, Joe and I wrote an add-on that adds something similar last week: https://github.com/wbamberg/gcli-mdn. It uses the MDN JSON search API (that was discussed in bug 768456 comment 17) to get information on the property. I don't know if that's better or not, but if you can use any of it, you're most welcome to!
Depends on: 977532
Comment on attachment 8381686 [details] [diff] [review]
769572-cssdoc-glci.patch

Review of attachment 8381686 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!, looks good so far.

I was at a meet-up with Will last week, and a side project of his work was the hack that he posted. I'm sure we can make something from our contributions. One thing that you might find of value is splitting the execution of the command from the formatting of the output. This is good for a number of reasons:

* If we are able to pipe data from one command to another, working on the raw data is a much better idea
* Raw data is more easily JSON-able, so commands can be run in separate processes more easily.

I'm not sure why the clicks are not working. I raised bug 977532 to have a look at that.

We'll need tests. browser_cmd_cookie.js [1] is a good example of some tests, and there are docs on helpers.audit in the source [2].

[1]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/browser_cmd_cookie.js
[2]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/helpers.js#982
Attachment #8381686 - Flags: review?(jwalker) → review+
I thought of something else. This is going to be hard to test because network resources are not available to all test boxes, so we can't make a call to MDN.

I think the solution is to allow the url to be customizable. Something like this:

  params: [
    {
      name: 'property',
      type: 'string',
    },
    {
      name: 'url',
      type: 'string',
      hidden: true,
      defaultValue: 'https://developer.mozilla.org/en-US/search.json?q=${property}&topic=css'
    }
  ],

  exec: function(args, context) {
    var url = args.url.replace(/${property}/, args.property);
    ...
  }

Then in the test you can say:

function test() {
  helpers.addTabWithToolbar(TEST_URI, function(options) {
    return helpers.audit(options, [
      // ...
      {
        setup: "cssdoc color --url http://example.com/browser/browser/devtools/commandline/test/browser_cmd_cssdoc_${property}.json",
        exec: {
          output: ...
        }
      },
      // ...
    ]);
  }).then(finish);
}
  
Then you can fake a response with a file alongside the tests called browser_cmd_cssdoc_color.json

BTW: The direct link for Will's hack is here https://github.com/wbamberg/gcli-mdn/blob/master/lib/main.js#L25
Joe, thanks for the pointers. I will add tests and make the command configurable. First, I'd like to discuss this:

(In reply to Will Bamberg [:wbamberg] from comment #18)
> In case it's useful, Joe and I wrote an add-on that adds something similar
> last week: https://github.com/wbamberg/gcli-mdn. It uses the MDN JSON search
> API (that was discussed in bug 768456 comment 17) to get information on the
> property. I don't know if that's better or not, but if you can use any of
> it, you're most welcome to!

I can definitely borrow from that code, thanks! Seems like the main question is: what do we want to actually display when they use the cssdoc command?

Do we want to display the title, excerpt, and link from the first result from the JSON API? That does seem like a better thing to do than what we are currently doing, which is to hit the page /CSS/<name> and see what turns up. The JSON API is probably faster because we don't have to wait for redirects, and it has the benefit of being a real search. So there will be results more often (for example sending "display2" to the JSON API still returns the right excerpt for the "display" property).

The excerpt does seem to be a little more terse than grabbing the summary from the page, but that's just from a quick glance on a few searches. More than likely the excerpt is better anyway because it's not just the first paragraph from the MDN page.
Attached patch 769572-cssdoc-glci.patch (obsolete) — Splinter Review
I really like using the JSON API, especially since it supports CORS. This is basically Will's code. I made the URL configurable.

The "click for more" link doesn't work. I can't figure out why links in the output box doesn't work. Something must be erroring because when I click the link, the box breaks. It won't show up in any future invokes of "cssdoc".

That seems like the last issue to fix. Any ideas?
Attachment #8381686 - Attachment is obsolete: true
I like your changes to the code - thanks.

There's been some discussion on the mailing list about this - the question is the safety of allowing publicly editable stuff to be shown as part of developer tools.

https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/zOvAau_6x7k

Some of this is about more automatic pop-ups but it's worth reading.

(In reply to James Long (:jlongster) from comment #22)
> The "click for more" link doesn't work. I can't figure out why links in the
> output box doesn't work. Something must be erroring because when I click the
> link, the box breaks. It won't show up in any future invokes of "cssdoc".
> 
> That seems like the last issue to fix. Any ideas?

I raised bug 977532 to fix it.
(In reply to Joe Walker [:jwalker] from comment #23)
> There's been some discussion on the mailing list about this - the question
> is the safety of allowing publicly editable stuff to be shown as part of
> developer tools.
> 
> https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/
> zOvAau_6x7k

I've thought about the l10n concerns. I also think it's not the best UX to use XHR because there's no loader indicator. I don't think you can ever depend on a network request without a loading indicator. Otherwise if MDN is being slow one day, the user it left wondering what the heck is going on.

Honestly, IMHO I wouldn't really use this command. What I would use a ton is an MDN search feature. So instead I would say something like "mdncss display" and it would open up https://developer.mozilla.org/en-US/search?q=display&topic=css in a new tab. Note how it restricted the search to the CSS topic.

I think that's a lot easier, more maintainable, and provides about as much usefulness.


> (In reply to James Long (:jlongster) from comment #22)
> I raised bug 977532 to fix it.

Do no other glci popups have links?
(In reply to James Long (:jlongster) from comment #24)
> (In reply to Joe Walker [:jwalker] from comment #23)
> > There's been some discussion on the mailing list about this - the question
> > is the safety of allowing publicly editable stuff to be shown as part of
> > developer tools.
> > 
> > https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/
> > zOvAau_6x7k
> 
> I've thought about the l10n concerns.

MDN does locale negotiation, so I think we're OK there.

> I also think it's not the best UX to
> use XHR because there's no loader indicator. I don't think you can ever
> depend on a network request without a loading indicator. Otherwise if MDN is
> being slow one day, the user it left wondering what the heck is going on.

GCLI should handle this automatically and supply a spinner for you although I think right now there is no spinner.

> Honestly, IMHO I wouldn't really use this command. What I would use a ton is
> an MDN search feature. So instead I would say something like "mdncss
> display" and it would open up
> https://developer.mozilla.org/en-US/search?q=display&topic=css in a new tab.
> Note how it restricted the search to the CSS topic.
> 
> I think that's a lot easier, more maintainable, and provides about as much
> usefulness.

I think this is effectively the same thing though - it is a search - see the response.documents[0] in the output formatter.
We could easily loop over the found results - See https://github.com/mozilla/domtemplate for docs on how looping works.
Summary: GCLI needs an CSSDOC command → GCLI needs a CSSDOC command
(In reply to Joe Walker [:jwalker] from comment #25)
> GCLI should handle this automatically and supply a spinner for you although
> I think right now there is no spinner.

Any idea why? I definitely don't see one.

> I think this is effectively the same thing though - it is a search - see the
> response.documents[0] in the output formatter.
> We could easily loop over the found results - See
> https://github.com/mozilla/domtemplate for docs on how looping works.

Yeah, it is basically the same thing which why I would opt for the other behavior of just dropping into the actual MDN search page. It's about the same thing with a lot less UX/maintenance/etc work. But that's my opinion, I'll glady continue implementing this as you all see fit.
MDN is not behaving right now. However I updated some of the error handling while I was trying to work out what was wrong.
Attachment #8384904 - Attachment is obsolete: true
Resetting priorities because these P* levels are very out of date.
Sorry for the bug spam. Filter on Lobster Thermidor
Priority: P2 → --
Don't think I'll be working on this anytime soon.
Assignee: jlong → nobody
Mentor: jwalker
Whiteboard: [gclicommands][good first bug][mentor=jwalker] → [gclicommands][good first bug]
Hi, can I work on this bug ?
(In reply to Sébastien Blin [:sblin] [:amarok] from comment #30)
> Hi, can I work on this bug ?

Of course, assigning the bug to you.

Ping jwalker on the #devtools channel at irc://irc.mozilla.org if you have any questions.
Assignee: nobody → amarok
(In reply to Sébastien Blin [:sblin] [:amarok] from comment #30)
> Hi, can I work on this bug ?

Thanks for the interest.

The latest source that I know of is in comment 27.
Here are the things that I think we need to do:

* We need to decide if we're going to have one command or many. I think we're probably best just have one command, so we should dup bug 769572 to this one.
* Assuming just one command, ee need a way to decide if this is a CSS lookup or a JS one, or others. I'll do a separate comment on this, but for here we'd want a new param which is something like { name: "topic", type: { name: "selection", data: [ "js", "css" ] } } and we then use args.topic rather than "css" in the XHR call.
* BuiltinCommands.jsm was split up into separate files, so we'll need to convert this in the same way See toolkit/devtools/gcli/commands. I think cookie.js [1] is a decent example. The new file will need registering in commands-index.js [2].
* We'll need tests. The cookie tests [3] are again an example, they'll need registering too [4], and the docs for helpers.audit [5] are probably useful.

Other relevant bugs:

* Bug 989063 is for when we're including MDN content in Firefox not in a web page. Worth reading. We might want to skip the step of showing an initial snippet for now as a result.
* Bug 768456 is the MDN side of this. It's open, but probably should be closed. Worth taking a quick look through.

[1]: http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/gcli/commands/cookie.js
[2]: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/commands-index.js
[3]: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/browser_cmd_cookie.js
[4]: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/browser.ini
[5]: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/helpers.js#1103
Will, bug 768456 comment 17 talks about topics, and how to get a list of them. I thought that a topic was an MDN tag, but now I'm not sure (because for JavaScript, the topic seems to be "JS", but the tag is "JavaScript").
How do we get a list of topics?
Thanks.
Flags: needinfo?(wbamberg)
@jwalker :
Thank for the comment. 
Ok for the last patch.

"* We need to decide if we're going to have one command or many. I think we're probably best just have one command, so we should dup bug 769572 to this one." -> Personally, I think one command is better. We can imagine a command like "mdn lang request" for example.

I am currently reading your tutorial about GCLI.
The search API supports a dynamic series of filters that can be retrieved from the JSON search result set. The filters aren't published standalone right now.

Check out the "filters" list in https://developer.mozilla.org/en-US/search.json?q=

This list contains filter groups with their various options. E.g. one of the filter groups is "Topics". 

Each of the filter group options has:

- a verbose "name"
- a "slug" to be used in the query parameter (together with the slug of the filter group)
- the "count" for that filter in the current search
- a boolean whether this particular filter is currently "active"
- two URLs with the filter active and inactive for the current search

That should give you enough information to programmatically use those filters in the CLI. Make sure to not hardcode the filter groups and their options as this is a dynamic system and can be changed by editors and MDN admins.
Sigh, of course I just see by clicking on the link above that the search view returns a 500 if you only request https://developer.mozilla.org/en-US/search.json?q *sigh*

Make sure you at least have an empty query parameter (with the equal sign).
Flags: needinfo?(wbamberg)
Hi,
Just to say I'm currently working on this bug.
So I have some questions.
1: In the patch v3 jlong use "browser/devtools/commandline/BuiltinCommands.jsm" File. What is this file. I have not got this file into my directory ?
2: In my patch, I replace "response.documents[0].link" by "response.documents[0].url" because link does not exists in the JSON API
3: I add a language parameter because we can search with topic=
4: My principal question. In the patch v3, jlong show the excerpt param. I think it is not sufficient. For example if I search "mdn transform" I obtain :
"The CSS transform property lets you modify the coordinate space of the CSS visual formatting model. Using" (You can see this page :https://developer.mozilla.org/en-US/search?q=transform&topic=css). So I think we can get the HTML of the first result (https://developer.mozilla.org/en-US/docs/Web/CSS/transform for example), and then get the content of the meta "description". It is more complex but returns a better result.
What do you think of this idea ?
Flags: in-moztrap?(jre33p)
Flags: a11y-review+
Flags: in-moztrap?(jre33p)
Flags: a11y-review+
No updates in many months, unassigning.
Assignee: amarok → nobody
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: