Closed Bug 298398 Opened 19 years ago Closed 19 years ago

Only single-click is needed for opening a History item, not double-click

Categories

(Firefox Graveyard :: Help Documentation, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Firefox1.5

People

(Reporter: jwalden+fxhelp, Assigned: hendrik)

References

()

Details

Attachments

(1 file, 1 obsolete file)

See bug 279506 comment 22.  The faulty reference is in using_firebird.xhtml in
the last list item underneath Retracing Your Steps (searching for "History"
finds it quite nicely).
A few comments on the patch (attachment 186962 [details] [diff] [review], for those who didn't see the
initial comment in another bug):

First, you don't need that first change to move the colon inside the <strong/>
element.  The style you saw there is the style we use throughout Help to the
best of my memory, and it's not going to be changed.

Now, on to the meat of the patch.  The removal of "double-" is fine, but
rereading it makes me think there are a few changes we should make in this area
while we're fixing stuff.  First, I'm not sure why we're telling the user to
click on the URL.  Most pages have titles, so we're targeting the edge case here
(no title or not HTML) when we should be targeting the common case (a title).  I
think we should change "URL" to "page title", which would make this more
accurate.  Second, I don't know why we're referring to "Bookmark" here anyway,
because we're dealing with "History" entries.  Maybe switching "the Bookmark"
for "the page's" would do the trick.  Steffen, what do you think of this?

Anyway, edit the patch as described above and post the new version, setting the
review flag to "steffen.wilberg".  After he reviews it I think we should be good
to go.
(In reply to comment #1)
> A few comments on the patch (attachment 186962 [details] [diff] [review] [edit], for those who didn't
see the
> initial comment in another bug):
> 
> First, you don't need that first change to move the colon inside the <strong/>
> element.  The style you saw there is the style we use throughout Help to the
> best of my memory, and it's not going to be changed.

Ok, I just thought it looked better that way.  There is a colon inside the
<strong/> on line 67 though...

> Now, on to the meat of the patch.  The removal of "double-" is fine, but
> rereading it makes me think there are a few changes we should make in this area
> while we're fixing stuff.  First, I'm not sure why we're telling the user to
> click on the URL.  Most pages have titles, so we're targeting the edge case here
> (no title or not HTML) when we should be targeting the common case (a title).  I
> think we should change "URL" to "page title", which would make this more
> accurate.  Second, I don't know why we're referring to "Bookmark" here anyway,
> because we're dealing with "History" entries.  Maybe switching "the Bookmark"
> for "the page's" would do the trick.  Steffen, what do you think of this?
> 
> Anyway, edit the patch as described above and post the new version, setting the
> review flag to "steffen.wilberg".  After he reviews it I think we should be good
> to go.

I follow you on all this stuff, and I'm willing to make a patch for this, but
I'm not a native speaker, so feedback is needed.

Isn't there a general place where I can report minor bugs like this?  I
encounter quite a few of them, as I'm working on the Dutch translation of these
files, and it would be handy if there were just one bug where I could mention
them, and I would prefer a native speaker (or someone with more self-confidence
concerning English) fixing them.  I'm thinking of something similar to bug
291678 (you won't understand most of it, as it is in Dutch, but it is a point
where several people post patches and discuss them, concerning various points in
the localisation)
Status: NEW → ASSIGNED
(In reply to comment #2)
> Isn't there a general place where I can report minor bugs like this?

Generally I think the consensus is one bug per issue, regardless how big or
small the issue is.  (Of course, if there are a few small bugs all dealing with
the same general issue, then filing one bug with a good description is better.)
 Multiple bugs make tracking fixes easier, and it makes patches smaller and
easier to read (as opposed to monster patches which fix a lot of things but
which are hard to read and review).

> I encounter quite a few of them, as I'm working on the Dutch translation of
> these files, and it would be handy if there were just one bug where I could
> mention them, and I would prefer a native speaker (or someone with more
> self-confidence concerning English) fixing them.

I'd prefer if you'd file separate bugs for all the issues, but it's not a
problem if you don't want to do the patches.

As to why I assigned this bug to you, I figured since you'd already provided a
patch for the base issue you'd be fine adding in a little extra.  In the future
one of the English Firefox Help people can write the patches if that's how you
prefer it.
Attached patch requested changes (obsolete) — Splinter Review
I moved the colon back to its place, although I still think this is
inconsistent behaviour.

I removed the reference to bookmark icons altogether, as it displays an empty
page most (all?) of the time anyway.  Removed Double- for clicking the folders
too.

I'll enter new bugs for similar small bugs, and maybe from time to time send in
a patch, but I'd really like reviews, and won't always be able to accept
assignments.
Attachment #187016 - Flags: review?(steffen.wilberg)
Comment on attachment 187016 [details] [diff] [review]
requested changes

>+    Clicking the folders displays subfolders or titles of web pages.
>+    You can click the page title to view that page.</li>
r=me, with "the page title" replaced by "a page's title".
"a page title" would be ok as well.
Attachment #187016 - Flags: review?(steffen.wilberg)
Attachment #187016 - Flags: review+
Attachment #187016 - Flags: approval-aviary1.1a2?
Attached patch more changesSplinter Review
Ok, I added Stephen's comment.
Attachment #187016 - Attachment is obsolete: true
Comment on attachment 187032 [details] [diff] [review]
more changes

Thanks for the patch. I could've fixed that myself on checkin though. Please
set flags accordingly when attaching new patches. You can carry over an r+ and
request approval-aviary1.1x yourself.
Attachment #187032 - Flags: review+
Attachment #187032 - Flags: approval-aviary1.1a2?
Attachment #187016 - Flags: approval-aviary1.1a2?
(In reply to comment #7)
> (From update of attachment 187032 [details] [diff] [review] [edit])
> Thanks for the patch. I could've fixed that myself on checkin though. Please
> set flags accordingly when attaching new patches. You can carry over an r+ and
> request approval-aviary1.1x yourself.

I'm afraid this sounds like Chinese to me.  I'm rather new to the whole CVS and
bugzilla system...
Once the patch has approval, I'll save it to disk and apply it to my local tree
by "patch < patchfile". I'll rebuild Firefox to make sure it works properly.
Then I'll check it in by doing "cvs commit", and mark the bug as fixed.

I didn't need an updated patch because after applying the patch, I could have
opened using_firebird.xhtml in my text editor, make the necessary change, and
then check it in.

When you provide a new patch which only addresses reviewer's comments to patch
he already marked as review+, you would be allowed mark "review+" yourself.

Now if that's confusing, don't worry. You'll learn that soon enough by filing
and fixing bugs.
(In reply to comment #9)
> Once the patch has approval, I'll save it to disk and apply it to my local tree
> by "patch < patchfile". I'll rebuild Firefox to make sure it works properly.

Not really necessary here, as it is just some altered text, but thanks for
explaining the process.

> Then I'll check it in by doing "cvs commit", and mark the bug as fixed.
> 
> I didn't need an updated patch because after applying the patch, I could have
> opened using_firebird.xhtml in my text editor, make the necessary change, and
> then check it in.
> 
> When you provide a new patch which only addresses reviewer's comments to patch
> he already marked as review+, you would be allowed mark "review+" yourself.
> 
> Now if that's confusing, don't worry. You'll learn that soon enough by filing
> and fixing bugs.

I get it, thanks.
Attachment #187032 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Checking in mozilla/browser/locales/en-US/chrome/help/using_firebird.xhtml;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/using_firebird.xhtml,v  <-- 
using_firebird.xhtml
new revision: 1.24; previous revision: 1.23
done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: