Closed Bug 1514275 Opened 5 years ago Closed 5 years ago

After successfully filing or submitting changes to a bug, bugzilla sometimes tells me I need to log in

Categories

(bugzilla.mozilla.org :: General, defect, P2)

Product:
bugzilla.mozilla.org
Component:
General
Version:
Production
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: dylan, Assigned: imadueme)

References

Details

Attachments

(4 files)

Screenshot_2018-12-14 1514275 processed.png
560.79 KB, image/png
Details
screenshot of dholbert hitting this on landing page after posting a bug comment
336.70 KB, image/png
Details
Screenshot_2018-12-18 1515144 processed.png
245.15 KB, image/png
Details
Screenshot of login comment (Rob).png
64.34 KB, image/png
Details 
Pike: "I'm just coming across a " You need to log in before you can comment on or make changes to this bug. " footer after changing a bug. Still logged in OK"

"just a single line at the bottom, everything else looked normal. header also showed my logged in information"
Testing that commenting doesn't trigger this
Just happened to me while trying to CC myself.
Why doesn't this happen to me?
It only happens to me intermittently. I've seen it maybe three times total so far of all the edits I've done in the last few days.
I've hit this as well. [Updating summary to be a bit clearer.]
Summary: Strange message when filing a new bug → After successfully filing or submitting changes to a bug, bugzilla sometimes tells me I need to log in
Just happened to me as well after commenting on Bug 1515144.
I've been seeing this intermittently for the last few days. :-(

In terms of explaining comment #4, I've got comment ordering set to "newest to oldest but keep description at the top", and comment box "before other comments", in case that's something that could be involved...
(In reply to :Gijs (he/him) from comment #9)
> I've been seeing this intermittently for the last few days. :-(
> 
> In terms of explaining comment #4, I've got comment ordering set to "newest
> to oldest but keep description at the top", and comment box "before other
> comments", in case that's something that could be involved...

I don't have either of those settings modified from default.
FWIW this is happening much more frequently now.
Severity: normal → major
Priority: -- → P1
Oh good, this is really easy.

Given:

[% primes = [1,3,5,7,11] %]
[% prime = 13 %]
Prime: [% prime FILTER none %]
<ul>
[% FOREACH prime IN primes %]
<li>[% prime FILTER none %]
[% END %]
</ul>
Prime: [% prime FILTER none %]

The output will be:

 Prime: 13

    1
    3
    5
    7
    11 

Prime: 11
The above comment is showing that the foreach loop doesn't reset the variable to its initial value when the loop exits.
This seems to be a regression in the template library -- still testing this hypothesis.
Assignee: dylan → imadueme

To fix this and other bugs, Israel and I discussed doing a page redirect instead of attempting to render the show_bug page from multiple locations.

The notices at the top of the page (the sent emails stuff) can be stored in flash.
There would be only a single code path (from show_bug.cgi) to display the bug modal page.

I'm asking for a vote on this from the module owners and peers.

Flags: needinfo?(kohei.yoshino)
Flags: needinfo?(ehumphries)
Flags: needinfo?(dkl)

One path into show_bug sounds good so I'm happy with a redirect to a new page.

I'm assuming that flash here is a module in the framework and not Adobe's flash? 😎

Flags: needinfo?(ehumphries)

Yeah, it's session storage that lasts for one request.

Example usage:

# Show message after redirect
$c->flash(message => 'User created successfully!');
$c->redirect_to('show_user', id => 23);

So the plan is always redirecting to show_bug.cgi instead of rewriting the URL path with JavaScript? It sounds good, I think.

Flags: needinfo?(kohei.yoshino)

(In reply to Dylan Hardison [:dylan] (he/him) from comment #19)

To fix this and other bugs, Israel and I discussed doing a page redirect instead of attempting to render the show_bug page from multiple locations.

The notices at the top of the page (the sent emails stuff) can be stored in flash.
There would be only a single code path (from show_bug.cgi) to display the bug modal page.

I'm asking for a vote on this from the module owners and peers.

+1

Flags: needinfo?(dkl)
Summary: After successfully filing or submitting changes to a bug, bugzilla sometimes tells me I need to log in → Always redirect to show_bug.cgi after creating bug, updating, creating attachment, or updating attachment

Note: please make sure this change works with all preference settings such as show no bug, show next bug in my list, etc.

Good point, I'll keep that in mind, thanks dkl

Status: NEW → ASSIGNED
Depends on: 1524174
Depends on: 1524175
Priority: P1 → P2

Any update here? This is still happening and is still annoying.

The only reason this is blocked is because after updating a bug bugzilla shows how many people were emailed as well as the email address of each person. With the redirect flow, on popular bugs we are unable to fit the entire recipient list into the user's session cookie until Bug 1525049 is completed. I chatted with dylan and I proposed that we for now (or permanently) simply display the number of people emailed and dropped the entire list of every email address so that we can get these changes out sooner.

The proposal is to: drop the full recipient list that displays when a bug is updated and only show the number of people emailed.

Dylan wanted to get input (vote) from those need info'd so please let us know if you think we should:

  1. do what is proposed until Bug 1525049 is completed
  2. do what is proposed permanently
  3. maintain current behavior and wait for Bug 1525049 to be completed
  4. other

Feedback is welcome from non-needinfo'd people too.

Flags: needinfo?(kohei.yoshino)
Flags: needinfo?(ehumphries)
Flags: needinfo?(dylan)
Flags: needinfo?(dkl)

I think we can simply drop it for good. I’m actually in the process of making email-related stuff less visible (Bug 179115, Bug 1525808, Bug 1525978), and such a recipient list will be gone anyway once bug form submissions are replaced with background REST API calls. Implementing a fixed bug header (Bug 1330451) is another reason to remove the list.

Flags: needinfo?(kohei.yoshino)

(In reply to Israel Madueme [:imadueme] from comment #30)

The only reason this is blocked is because after updating a bug bugzilla shows how many people were emailed as well as the email address of each person. With the redirect flow, on popular bugs we are unable to fit the entire recipient list into the user's session cookie until Bug 1525049 is completed. I chatted with dylan and I proposed that we for now (or permanently) simply display the number of people emailed and dropped the entire list of every email address so that we can get these changes out sooner.

The proposal is to: drop the full recipient list that displays when a bug is updated and only show the number of people emailed.

Dylan wanted to get input (vote) from those need info'd so please let us know if you think we should:

  1. do what is proposed until Bug 1525049 is completed
  2. do what is proposed permanently
  3. maintain current behavior and wait for Bug 1525049 to be completed
  4. other

Feedback is welcome from non-needinfo'd people too.

I vote also for #2. The only time the full recipient list has ever been useful is when people used to complain that people are getting emailed about bug changes that should not be. This was almost always due to people 'watching' another email account or components, etc. After explaining to them that Bugzilla does a good job of making sure that the wrong people can't see a private bug they were good. So if we just remove the recipient list that will do away with those types of questions. Otherwise no-one probably looks at that list anyway.

dkl

Flags: needinfo?(dkl)

(In reply to David Lawrence [:dkl] from comment #32)

Otherwise no-one probably looks at that list anyway.

Apologies for advocacy. FWIW, I look at it pretty religiously, as a doublecheck for possible comms misses (as in, "huh, I should tell so-and-so"). I would hope, if you're going to hide it, it's at least clickable to expand/get the list.

(In reply to Greg Cox [:gcox] from comment #33)

(In reply to David Lawrence [:dkl] from comment #32)

Otherwise no-one probably looks at that list anyway.

Apologies for advocacy. FWIW, I look at it pretty religiously, as a doublecheck for possible comms misses (as in, "huh, I should tell so-and-so"). I would hope, if you're going to hide it, it's at least clickable to expand/get the list.

Apologize for being too broad in my statement. Maybe we could do some metrics to figure out how many people click on the show link to display the full list. Trying to remember if the default is to hide the list and just show the number of recipients.

If we still provide a way to show the full list when expanded then we will have to go ahead with option 1.

@kohei, what about using localstorage for the email list keyed to the delta_ts timestamp of the update?

dkl

I think #2 is the right way to go, and we need to rethink component monitoring. There's better ways to do that than email.

Flags: needinfo?(ehumphries)

We cannot use local storage here because the redirect will be done server-side, not with JavaScript. Only cookies and server-side storage can be used, but I don’t think it’s necessary because, as I said, the list will be gone sooner or later once the bug page becomes Ajax-y.

(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #36)

We cannot use local storage here because the redirect will be done server-side, not with JavaScript. Only cookies and server-side storage can be used, but I don’t think it’s necessary because, as I said, the list will be gone sooner or later once the bug page becomes Ajax-y.

Ah ok. yeah the ajax request would just get the list of recipients back from the request and then display them if the user chooses to. Long term that is definitely the nicest option.

My longer goal is making Bugzilla a self-contained app like BzDeck, Gmail, Slack or whatever, and killing email notifications completely 😉 (Sending digest mails is fine)

Blocks: 1330451

#2 is good for me, but I hear gcox's opinions too. I'm going to pretend comment 38 never happened because that sounds like a disaster for me.

Blocks: 1533582
No longer blocks: 1330451

I do hear gcox's concerns, but due to the other changes Kohei mentioned option #2 is the path forward.

Flags: needinfo?(dylan)
Type: defect → task
Type: task → defect
Blocks: 1330451

(Reverting the Summary as the fix is being baked in the dependency bugs and the type of this bug is defect.)

Summary: Always redirect to show_bug.cgi after creating bug, updating, creating attachment, or updating attachment → After successfully filing or submitting changes to a bug, bugzilla sometimes tells me I need to log in
No longer blocks: 1533582
No longer blocks: 1330451

This is still happening, quite frequently for me in the last couple of days. It's expanded in scope, so now when I type in a comment and hit submit, it will "log me out" before processing the submission, giving me a variety of errors related to expired/mismatched tokens. So the bit of this bug's summary that says "After successfully filing or submitting changes" is no longer accurate, as it actually prevents me from successfully filing or submitting changes.

The token issue is probably different, reported as Bug 1559445.

It doesn't sound quite like that bug as reported, but it might be the same underlying problem with cookies not being sent. I do notice that when I get this error the bugzilla header bar shows me as logged out rather than logged in. I'll grab a screenshot next time it happens.

See Also: → 1549637

Only just noticed it since yesterday (pretty sure I would have noticed before if it had happened).

Also, not sure if it's related, but at 27 Jun 2019 20:15:54 +0000 I received an email about a needinfo, but I can't see it anywhere on Bugzilla (in bug 1561393, nor in my NI queue).

This has definitely gotten worse since all-hands. I see it about 5 times per day now.

I'm running into this more often lately. Definitely true in the last week or so.

:imadueme’s PRs have been merged to master, will be deployed in the next production push.

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

The fix deployed a couple of minutes ago.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: