If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Create reload marker in the Web Console

RESOLVED FIXED in Firefox 25

Status

()

Firefox
Developer Tools: Console
P2
enhancement
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: rc, Assigned: msucan)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Web Console should have an indicator to mark where reload or navigation occurred in the output area.

Spin off from bug 705921.
(Reporter)

Updated

5 years ago
Depends on: 705921
(Assignee)

Updated

5 years ago
Blocks: 778766
(Assignee)

Updated

5 years ago
Priority: -- → P2
(Assignee)

Updated

4 years ago
Duplicate of this bug: 879689
(Assignee)

Comment 2

4 years ago
Started work on this patch.
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
(Assignee)

Comment 3

4 years ago
Created attachment 771417 [details] [diff] [review]
proposed patch

This is the proposed patch.

Screenshot: http://img.i7m.de/show/drlpt.png

Notes and concerns:

- the new ConsoleOutput needs to be a weird cross between the current web console code and the new API. I did minimal bits of the proposed API for ConsoleOutput, BaseMessage and NavigationMarker (renamed from ReloadMarker).

- filtering, removal, and all of the other aspects related to output are still handled by the main web console code. Tried to keep the patch small. We will move more functionality over to the new APIs in other bugs and patches.

- the navigation marker behaves as a kind of a network message, which means it is filtered and pruned in the same way. Ideally, the marker would be a visual indicator of a group of all kinds of messages that happen between different page navigations. One that is not pruned until its last child is pruned, and so on.

I did avoid diving into a potential fix for this issue. We need more of the new APIs before we can do this in a good way. We can do it later in bug 655700.


Looking forward to your comments. Thank you!


Patch queue: bugs 887273, 812618, 877262 then this one.
Attachment #771417 - Flags: review?(rcampbell)
Whoa!
(Assignee)

Comment 5

4 years ago
This patch removes WCF_regroupOutput() which wasn't useful for a long time, thus fixing bug 746871.
Blocks: 746871
(Assignee)

Updated

4 years ago
Blocks: 859841
(Assignee)

Updated

4 years ago
Blocks: 760876
(Assignee)

Comment 6

4 years ago
Created attachment 775702 [details] [diff] [review]
rebased patch
Attachment #771417 - Attachment is obsolete: true
Attachment #771417 - Flags: review?(rcampbell)
Attachment #775702 - Flags: review?(rcampbell)
(Reporter)

Comment 7

4 years ago
Comment on attachment 775702 [details] [diff] [review]
rebased patch

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

::: browser/devtools/webconsole/console-output.js
@@ +72,5 @@
> +   * @param object message...
> +   *        Any number of Message objects.
> +   * @return this
> +   */
> +  addMessage: function()

no parameters here.

if you want to use var args style arbitrary arguments, you can use rest parameters.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/rest_parameters

e.g., addMessage: function(...args)

@@ +74,5 @@
> +   * @return this
> +   */
> +  addMessage: function()
> +  {
> +    for (let i = 0; i < arguments.length; i++) {

could use for...of with rest parameters! (ok, I'll shut up about rest parameters now, they're pretty-much equivalent except arguments isn't a real array)

@@ +327,5 @@
> +      continue;
> +    }
> +
> +    result["$" + key] = fn;
> +  }

Asked Irakli about this and his recommendation is to call the parent object's prototype method directly where needed.

For example, from https://addons.mozilla.org/en-US/developers/docs/sdk/1.14/modules/sdk/core/heritage.html

var Pet = Class({
  extends: Dog,              // should inherit from Dog
  initialize: function initialize(breed, name) {
    // To call ancestor methods you will have to access them
    // explicitly
    Dog.prototype.initialize.call(this, name);
    this.breed = breed;
  },
  call: function call(name) {
    return this.name === name ? this.bark() : '';
  }
});


In places where you're going to need to call the super method, you can give it a convenient alias.

Just sayin', I don't think it's necessary to really wrapper Heritage.extend().
Attachment #775702 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 8

4 years ago
(In reply to Rob Campbell [:rc] (:robcee) from comment #7)
> Comment on attachment 775702 [details] [diff] [review]
> rebased patch
> 
> Review of attachment 775702 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/webconsole/console-output.js
> @@ +72,5 @@
> > +   * @param object message...
> > +   *        Any number of Message objects.
> > +   * @return this
> > +   */
> > +  addMessage: function()
> 
> no parameters here.
> 
> if you want to use var args style arbitrary arguments, you can use rest
> parameters.
> 
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> rest_parameters
> 
> e.g., addMessage: function(...args)

Good catch. I forgot about the rest parameters. Fixing this in the updated patch.


> @@ +74,5 @@
> > +   * @return this
> > +   */
> > +  addMessage: function()
> > +  {
> > +    for (let i = 0; i < arguments.length; i++) {
> 
> could use for...of with rest parameters! (ok, I'll shut up about rest
> parameters now, they're pretty-much equivalent except arguments isn't a real
> array)
> 
> @@ +327,5 @@
> > +      continue;
> > +    }
> > +
> > +    result["$" + key] = fn;
> > +  }
> 
> Asked Irakli about this and his recommendation is to call the parent
> object's prototype method directly where needed.
> 
> For example, from
> https://addons.mozilla.org/en-US/developers/docs/sdk/1.14/modules/sdk/core/
> heritage.html
> 
> var Pet = Class({
>   extends: Dog,              // should inherit from Dog
>   initialize: function initialize(breed, name) {
>     // To call ancestor methods you will have to access them
>     // explicitly
>     Dog.prototype.initialize.call(this, name);
>     this.breed = breed;
>   },
>   call: function call(name) {
>     return this.name === name ? this.bark() : '';
>   }
> });
> 
> 
> In places where you're going to need to call the super method, you can give
> it a convenient alias.
> 
> Just sayin', I don't think it's necessary to really wrapper
> Heritage.extend().

Good point. Better to use what others also use, for consistency. Fix coming in the updated patch.
(Assignee)

Comment 9

4 years ago
Created attachment 785373 [details] [diff] [review]
address review comments and rebase
Attachment #775702 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Green try push: https://tbpl.mozilla.org/?tree=Try&rev=0258b12aba0b

The push includes this patch and the patches from bug 877262.
(Assignee)

Comment 11

4 years ago
Landed: https://hg.mozilla.org/integration/fx-team/rev/9a1e79294a20
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/9a1e79294a20
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.