The default bug view has changed. See this FAQ.

Port |Bug 451833 - Highlight the domain name in the address bar| to Seamonkey

RESOLVED FIXED in seamonkey2.10

Status

SeaMonkey
Location Bar
--
enhancement
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Phoenix, Assigned: neil@parkwaycc.co.uk)

Tracking

({ux-visual-hierarchy})

Trunk
seamonkey2.10
ux-visual-hierarchy
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.10 fixed)

Details

(Whiteboard: [parity-Firefox][parity-IE][parity-chrome][parity-opera])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
What about porting mentioned bug to Seamonkey?
(Reporter)

Updated

6 years ago
Severity: normal → enhancement
Keywords: ux-visual-hierarchy

Comment 1

6 years ago
SeaMonkey developers are unlikely to work on this feature. However if someone supplies a suitable patch we won't reject it out of hand.

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

5 years ago
Duplicate of this bug: 184074
(Reporter)

Comment 3

5 years ago
Created attachment 602313 [details] [diff] [review]
Just add setting for it

I checked sources and found, that almost all needed code already here in shared components, and only pref for highlighting missing, proposing trivial patch
Attachment #602313 - Flags: review?
(Assignee)

Comment 4

5 years ago
Comment on attachment 602313 [details] [diff] [review]
Just add setting for it

Sorry, but browser/base/content/urlbarBindings.xml isn't a shared component.
Attachment #602313 - Flags: review? → review-
(Reporter)

Comment 5

5 years ago
Created attachment 602334 [details] [diff] [review]
Changes in suite/browser/urlbarBindings.xml

2nd file
Attachment #602334 - Flags: review?(neil)
(Assignee)

Comment 6

5 years ago
Comment on attachment 602334 [details] [diff] [review]
Changes in suite/browser/urlbarBindings.xml

It's mostly OK but it's missing a few bits. Tweaked patch coming up...
Attachment #602334 - Flags: review?(neil) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 602373 [details] [diff] [review]
Tweaked patch

Tweaks:
* Actually hook up a caller (!)
* Make the pref very live
* Reformat the URLbar after a blur
* Combine the two methods
Assignee: nobody → neil
Attachment #602313 - Attachment is obsolete: true
Attachment #602334 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #602373 - Flags: review?(philip.chee)

Comment 8

5 years ago
Comment on attachment 602373 [details] [diff] [review]
Tweaked patch

This patch works. One minor UI nit. If you have dynamic skin switching on and switch themes using View->Apply Theme, the highlighting disappears.

Some coding nits:
> +          } else if (aPref == "browser.urlbar.formatting.enabled") {
> +            this._formattingEnabled = this.mPrefs.getBoolPref("browser.urlbar.formatting.enabled");
How about?
            this._formattingEnabled = this.mPrefs.getBoolPref((aPref);

> +      <method name="_formatValue">
> +        <parameter name="formattingEnabled"/>
> +        <body><![CDATA[
> +          let controller = this.editor.selectionController;
I know this is just copy and paste from Firefox, but I note that this file doesn't use let's in general. So please replace all these let's with var's wherever applicable.

> +              ["http:", "https:", "ftp:"].indexOf(protocol[0]) == -1)
!/^(https?|ftp):/.test(protocol[0]);

> +            let segments = function (s) s.replace(/[^.]*/g, "").length + 1;
function (s) { .... };
Do we need a non-anonymous function here?
function segments(s) {...};

> +      <handler event="blur" phase="capturing"><![CDATA[
Is capturing needed here?
Attachment #602373 - Flags: review?(philip.chee)

Comment 9

5 years ago
> +      <handler event="ValueChange"><![CDATA[
> * Actually hook up a caller (!)

I wonder why Dão put a Firefox urlbarBinding specific caller in toolkit autocomplete.

Updated

5 years ago
Depends on: 451833
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [parity=Firefox][parity-IE][parity-chrome][parity-opera]
(Assignee)

Comment 10

5 years ago
(In reply to Philip Chee from comment #8)
> This patch works. One minor UI nit. If you have dynamic skin switching on
> and switch themes using View->Apply Theme, the highlighting disappears.
OK, I didn't think of that. Let me see what I can do.

> Some coding nits:
> > +          } else if (aPref == "browser.urlbar.formatting.enabled") {
> > +            this._formattingEnabled = this.mPrefs.getBoolPref("browser.urlbar.formatting.enabled");
> How about?
>             this._formattingEnabled = this.mPrefs.getBoolPref((aPref);
Well, I guess I should change the other prefs in that case...

> > +      <method name="_formatValue">
> > +        <parameter name="formattingEnabled"/>
> > +        <body><![CDATA[
> > +          let controller = this.editor.selectionController;
> I know this is just copy and paste from Firefox, but I note that this file
> doesn't use let's in general. So please replace all these let's with var's
> wherever applicable.
I did wonder about that, but then I thought that was getting too far away from the original patch...

> > +              ["http:", "https:", "ftp:"].indexOf(protocol[0]) == -1)
> !/^(https?|ftp):/.test(protocol[0]);
Fair enough.

> > +            let segments = function (s) s.replace(/[^.]*/g, "").length + 1;
> function (s) { .... };
> Do we need a non-anonymous function here?
> function segments(s) {...};
Good catch!

> > +      <handler event="blur" phase="capturing"><![CDATA[
> Is capturing needed here?
Yes, the blur event doesn't bubble. (The focusout event would, if we had it.)

(In reply to Philip Chee from comment #9)
> > > +      <handler event="ValueChange"><![CDATA[
> > * Actually hook up a caller (!)
> I wonder why Dão put a Firefox urlbarBinding specific caller in toolkit
> autocomplete.
So did I...
Whiteboard: [parity=Firefox][parity-IE][parity-chrome][parity-opera] → [parity-Firefox][parity-IE][parity-chrome][parity-opera]

Comment 11

5 years ago
From Bug 184074 Comment 26:
> Looks like IE8 is implementing this
> http://blogs.msdn.com/ie/archive/2008/03/11/address-bar-improvements-in-internet-explorer-8-beta-1.aspx

"Domain Highlighting will 'disappear' if the user hovers over or clicks in the Address Bar."
Neither we nor Firefox listen to hover. What does Chrome do?

Comment 12

5 years ago
>> Is capturing needed here?
> Yes, the blur event doesn't bubble. (The focusout event would, if we had it.)
The reason I asked is that the Firefox version doesn't use capturing.
(Assignee)

Comment 13

5 years ago
(In reply to Philip Chee from comment #8)
> (From update of attachment 602373 [details] [diff] [review])
> > +      <handler event="blur" phase="capturing"><![CDATA[
> Is capturing needed here?
In fact it isn't needed; smaug changed the way event handling worked, but incidentally caused an unrelated regression. Oops...
(Assignee)

Comment 14

5 years ago
Created attachment 602643 [details] [diff] [review]
Addressed review comments
Attachment #602373 - Attachment is obsolete: true
Attachment #602643 - Flags: review?(philip.chee)
(Reporter)

Comment 15

5 years ago
(In reply to Philip Chee from comment #11)
> Neither we nor Firefox listen to hover. What does Chrome do?
Firefox highlights address bar on hover, Chrome 17 do nothing

Comment 16

5 years ago
Comment on attachment 602643 [details] [diff] [review]
Addressed review comments

Cryptic comments strike again. When I said "replace all these let's with var's wherever applicable" I meant to imply that let's within sub-scopes shouldn't be changed. I'm not too torqued about this however. r=me
Attachment #602643 - Flags: review?(philip.chee) → review+
(Assignee)

Comment 17

5 years ago
Pushed changeset 5cb1917f9106 to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
I think this needs a matching option in Preferences. Please file a follow-up bug for that, depending on this one. In the meantime I set the relnote keyword so we can take care of at least explaining it (and how to disable it; I'm almost sure some of our more conservative users will ask).
Keywords: relnote
Target Milestone: --- → seamonkey2.10
(Assignee)

Comment 19

5 years ago
(In reply to Jens Hatlak from comment #18)
> I think this needs a matching option in Preferences. Please file a follow-up
> bug for that, depending on this one.
Yeah, I meant to do that already, but I forgot :-(
(Assignee)

Updated

5 years ago
Blocks: 732816

Updated

5 years ago
status-seamonkey2.10: --- → fixed
Keywords: relnote

Updated

4 years ago
Depends on: 823499
This bug here added some code which throws a JS error (this.editor is null) when opening a popup via window.open (maybe related to the popup not having an address bar?). See Bug 823499 for a testcase.
(In reply to Frank Wein [:mcsmurf] from comment #20)
> This bug here added some code which throws a JS error (this.editor is null)
> when opening a popup via window.open (maybe related to the popup not having
> an address bar?).

If that was the reason, maybe FF is not affected since it forces the address bar to be visible (but not editable) for popups.
You need to log in before you can comment on or make changes to this bug.