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)
Firefox Graveyard
Help Documentation
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox1.5
People
(Reporter: jwalden+fxhelp, Assigned: hendrik)
References
()
Details
Attachments
(1 file, 1 obsolete file)
980 bytes,
patch
|
steffen.wilberg
:
review+
asa
:
approval-aviary1.1a2+
|
Details | Diff | Splinter Review |
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).
Reporter | ||
Comment 1•19 years ago
|
||
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.
Assignee | ||
Comment 2•19 years ago
|
||
(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
Reporter | ||
Comment 3•19 years ago
|
||
(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.
Assignee | ||
Comment 4•19 years ago
|
||
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 5•19 years ago
|
||
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?
Assignee | ||
Comment 6•19 years ago
|
||
Ok, I added Stephen's comment.
Assignee | ||
Updated•19 years ago
|
Attachment #187016 -
Attachment is obsolete: true
Comment 7•19 years ago
|
||
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?
Updated•19 years ago
|
Attachment #187016 -
Flags: approval-aviary1.1a2?
Assignee | ||
Comment 8•19 years ago
|
||
(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...
Comment 9•19 years ago
|
||
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.
Assignee | ||
Comment 10•19 years ago
|
||
(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.
Updated•19 years ago
|
Attachment #187032 -
Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment 11•19 years ago
|
||
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
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•